Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Branko Čibej]

On 29.09.2015 00:41, Konstantin Boudnik wrote:
> Hmm...
>
> Negligence, n. : the trait of neglecting responsibilities and lacking concern
> syn : omission, oversight

"Negligence" usually means continued and repeated (non-)action. In that
respect it's an extremely negative label to use; you're effectively
accusing someone of being an incompetent lazy b**rd.

The "synonyms" are not synonymous because both "omission" and
"oversight" imply a on-time event.


-- Brane


> Doesn't sound catastrophic in my vocabulary, really. Does this
>> case of negligence and needs to be addressed accordingly.
> translate to "should face a firing squad without a trial of his peers"?
> Have I anywhere pointed a finger at you or anyone else? Or attacked someone? 
> Why are you all upset and defensive about it?
>
> Cos
>
> On September 28, 2015 7:39:51 AM PDT, Raul Kripalani  wrote:
>> Cos, your language seems too harsh for the situation.
>>
>> No one here is committing negligence. The explanation is simple: people
>> aren't perfect.
>>
>> Now, let's take a step back and see the big picture. Around 95% of the
>> commits in this project are by GridGain personnel (check git shortlog
>> -s
>> -n) who have spent months/years working on this codebase during their
>> daily
>> job. Their eyes are accustomed to this code style and naturally they'll
>> spot oddities in a twitch. It's obvious.
>>
>> For newer people, we don't even have checkstyle nor decent facilities
>> for
>> newer people to spot formatting issues quickly. Because, surprise! The
>> issues that Yakov spotted are simply of formatting. The code is
>> functional
>> and much better tested than other streamers and IP Finders. Other
>> streamers
>> have 1 test, this streamer has 9 unit tests! Look at the code.
>> Furthermore,
>> Yakov seems to have made a mistake reading the Git commit history.
>> There
>> were never WIP commits on master.
>>
>> So may I ask you to stop using catastrophic vocabulary. The situation
>> is
>> not catastrophic, it's simply improvable.
>>
>> Now, as an ASF member, I ask you to recognise that unaffiliated
>> volunteers
>> like me bring diversity to the project that's otherwise dominated by a
>> company. You should appreciate that – more so given that you're a
>> former
>> mentor. I do this for the fun, and attacks like yours take the fun out
>> of
>> it. Have a look again at this project's team composition and, for those
>> people not affiliated to GridGain, try to find when their last commit
>> was... Then you'll see what I mean.
>>
>> P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
>> Valentin will want to comment.
>>
>> Regards,
>>
>> *Raúl Kripalani*
>> PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
>> and
>> Messaging Engineer
>> http://about.me/raulkripalani |
>> http://www.linkedin.com/in/raulkripalani
>> http://blog.raulkr.net | twitter: @raulvk
>>
>> On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik 
>> wrote:
>>
>>> Are these official guidelines that are worked-out and communicated by
>>> community? Basically, were they made clear when the project went on
>> the CTR
>>> model? I presume it is/was looking at the wikipage. Hence
>> non-sticking
>>> to them is a case of negligence and needs to be addressed
>> accordingly.
>>> I would also want to highlight the other side of such negligence: by
>>> dumping
>>> semi-baked code to the master one creates a burden for the rest of
>> the
>>> community as the code degrades in quality, potentially breaks tests,
>> style
>>> checks, etc. And someone else needs to deal with it to unblock her's
>> future
>>> progress. And that's brings forward another point that Brane and I
>> were
>>> making on a few occasions: in the CTR communities you need to invite
>> in
>>> people
>>> with great deal of attention to how they work with others. Are they
>>> respecting
>>> others' time and effort? Are they good citizens of the community? And
>> on,
>>> and
>>> on.
>>>
>>> Another purely technically matter: master isn't a trash can. Master
>> should
>>> be
>>> close to releasable at any given point of time. WIP stuff doesn't
>> belong to
>>> master, that's what the dev and integration branches are for.
>>>
>>> Cos
>>>
>>> On Mon, Sep 28, 2015 at 03:31PM, Yakov Zhdanov wrote:
 Guys,

 I have just reviewed the code and have some comments. 1-2 are very
>>> serious
 from my point of view.

 1. Code is in master. Did anyone checked tests on TC? Moreover, are
>> there
 suites for those tests?
 2. It seems that work on streamer has been done directly in master.
>> I see
 WIP commits, but I think I should not. As agreed finished work
>> should be
 committed as a single set of changes.
 3. I see unused variable
 - org.apache.ignite.stream.mqtt.MqttStreamer#cachedLogPrefix
 4. Unused import - import com.google.common.base.Joiner;
 5. Code and javadocs lines exceed 120 chars restriction.
 6. 

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Anton Vinogradov]

Raul,

I've fixed most of TODOs you created.
Seems that indentation policy with multi-line parameter descriptions
already defined: "multiline comments in Javadoc tags should be indented by
4 or 5 characters ...".

Please check my changes:
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=57901455=16=15



On Tue, Sep 29, 2015 at 3:28 PM, Roman Shaposhnik 
wrote:

> On Tue, Sep 29, 2015 at 3:01 PM, Raul Kripalani  wrote:
> > Thanks, Cos. I'm glad we sorted this out. Sometimes a chat would be
> useful
> > to establish rapport.
> >
> > Accusing someone of negligence is *extremely* serious and casts a very
> bad
> > image on the person being attributed with it.
>
> I've been there and I understand where you're coming from. Different words
> have different level of gravity in different languages and cultures.
>
> FWIW: I didn't read it as invoking catastrophic ramifications and/or
> doubting
> integrity of the person. But that's *my* reading. Somebody who, at the end
> of
> the day, speaks English fluently but alas as a second language. I do
> apologize
> if it made if you feel uncomfortable.
>
> I think the key takeaway for all of us who chimed in on this thread is
> to be extremely
> appreciative of the fact that we all come from different backgrounds
> and cultures
> and can interpret things differently. Lets give each other a bit of a
> breathing
> room and a benefit of the doubt.
>
> Thanks,
> Roman.
>

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Dmitriy Setrakyan]

On Tue, Sep 29, 2015 at 2:01 PM, Raul Kripalani  wrote:

(3) we have no tooling in place. The Ignite community is dominated by
> the people who wrote the code and have become accustomed to reading it day
> and night for years. They have a trained eye to detect weird stuff. They
> need to understand that. Others like me do not and never will, because
> we're involved in tens of projects, each with a different coding style.
> That's why I insist in the importance of checkstyle. I'm bound to this
> community now, but if I was a newcomer, this kind of witch hunt would have
> deterred me from contributing to Ignite ever again. And as a current PMC
> member, I'm very concerned about this attitude in general (but that's a
> different topic).
>

Raul, I don't think I agree with you here. Somehow you are making it sound
that if we don't have the "right" tooling, then we should not try to
enforce coding guidelines. That would be a disaster in my view.

I think that given the current situation, providing styling comments during
reviews is the right way to go. All the feedback we have received so far is
that people generally appreciate how nice Ignite code looks, and gladly
accept the feedback on style from more experienced committers.

Also, take a look at some other replies on this subject, especially the
comments that came from the new community members.

D.

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Roman Shaposhnik]

On Tue, Sep 29, 2015 at 3:01 PM, Raul Kripalani  wrote:
> Thanks, Cos. I'm glad we sorted this out. Sometimes a chat would be useful
> to establish rapport.
>
> Accusing someone of negligence is *extremely* serious and casts a very bad
> image on the person being attributed with it.

I've been there and I understand where you're coming from. Different words
have different level of gravity in different languages and cultures.

FWIW: I didn't read it as invoking catastrophic ramifications and/or doubting
integrity of the person. But that's *my* reading. Somebody who, at the end of
the day, speaks English fluently but alas as a second language. I do apologize
if it made if you feel uncomfortable.

I think the key takeaway for all of us who chimed in on this thread is
to be extremely
appreciative of the fact that we all come from different backgrounds
and cultures
and can interpret things differently. Lets give each other a bit of a breathing
room and a benefit of the doubt.

Thanks,
Roman.

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Konstantin Boudnik]

Thank you you for confirming my point: there was a mistake and it needs to be
corrected. End of story. But instead of simply fixing it and moving on, we are
spending hours x 50 people on reading and writing long emails arguing about
imaginary semantical differences.

There's no need to be emotional about who said what: I deserve the benefits of
the doubt as well despite being a "former mentor of the project" whatever the
hell it means. I am not dismissing the value of your contributions to this
community: I welcome and appreciate your efforts! Neither have I targeted you
nor put your on the stand - you did it to yourself. If you don't like
something you think I addressed to you - send me a private email and explain
that I was a jerk and hurt your feelings: no need to make a public display of
potential nothingness. Would I choose to listen to it or not - is a separate
matter altogether: I have the same right to not read or accept something that
another person has wrote.

Let's move on. Best,
  Cos

On Tue, Sep 29, 2015 at 01:16AM, Raul Kripalani wrote:
> There has been no negligence, Cos! People are human and make mistakes. End
> of the story.
> 
> Bringing such negative verbiage to the community helps in nothing.
> Everybody is doing their best, I'd like to think so.
> 
> In fact, you have shifted the conversation away from the actual topic at
> hand. So thanks.
> 
> A suggestion: "Benefit of the doubt" is a powerful practice and keeps us
> away from errors in judgement.
> 
> With regards your list of questions, may I ask you to re-read your initial
> message. Don't make me explain what's obvious, mate.
> 
> Cheers.
> On 28 Sep 2015 23:41, "Konstantin Boudnik"  wrote:
> 
> > Hmm...
> >
> > Negligence, n. : the trait of neglecting responsibilities and lacking
> > concern
> > syn : omission, oversight
> >
> > Doesn't sound catastrophic in my vocabulary, really. Does this
> > > case of negligence and needs to be addressed accordingly.
> > translate to "should face a firing squad without a trial of his peers"?
> > Have I anywhere pointed a finger at you or anyone else? Or attacked
> > someone? Why are you all upset and defensive about it?
> >
> > Cos
> >
> > On September 28, 2015 7:39:51 AM PDT, Raul Kripalani 
> > wrote:
> > >Cos, your language seems too harsh for the situation.
> > >
> > >No one here is committing negligence. The explanation is simple: people
> > >aren't perfect.
> > >
> > >Now, let's take a step back and see the big picture. Around 95% of the
> > >commits in this project are by GridGain personnel (check git shortlog
> > >-s
> > >-n) who have spent months/years working on this codebase during their
> > >daily
> > >job. Their eyes are accustomed to this code style and naturally they'll
> > >spot oddities in a twitch. It's obvious.
> > >
> > >For newer people, we don't even have checkstyle nor decent facilities
> > >for
> > >newer people to spot formatting issues quickly. Because, surprise! The
> > >issues that Yakov spotted are simply of formatting. The code is
> > >functional
> > >and much better tested than other streamers and IP Finders. Other
> > >streamers
> > >have 1 test, this streamer has 9 unit tests! Look at the code.
> > >Furthermore,
> > >Yakov seems to have made a mistake reading the Git commit history.
> > >There
> > >were never WIP commits on master.
> > >
> > >So may I ask you to stop using catastrophic vocabulary. The situation
> > >is
> > >not catastrophic, it's simply improvable.
> > >
> > >Now, as an ASF member, I ask you to recognise that unaffiliated
> > >volunteers
> > >like me bring diversity to the project that's otherwise dominated by a
> > >company. You should appreciate that – more so given that you're a
> > >former
> > >mentor. I do this for the fun, and attacks like yours take the fun out
> > >of
> > >it. Have a look again at this project's team composition and, for those
> > >people not affiliated to GridGain, try to find when their last commit
> > >was... Then you'll see what I mean.
> > >
> > >P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
> > >Valentin will want to comment.
> > >
> > >Regards,
> > >
> > >*Raúl Kripalani*
> > >PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
> > >and
> > >Messaging Engineer
> > >http://about.me/raulkripalani |
> > >http://www.linkedin.com/in/raulkripalani
> > >http://blog.raulkr.net | twitter: @raulvk
> > >
> > >On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik 
> > >wrote:
> > >
> > >> Are these official guidelines that are worked-out and communicated by
> > >> community? Basically, were they made clear when the project went on
> > >the CTR
> > >> model? I presume it is/was looking at the wikipage. Hence
> > >non-sticking
> > >> to them is a case of negligence and needs to be addressed
> > >accordingly.
> > >>
> > >> I would also want to highlight the other side of such negligence: by
> > >> dumping
> > >> semi-baked code to the 

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Konstantin Boudnik]

Dmitriy has pointed to me why there's a incorrect perception of me going after
Raul ;) I haven't even noticed the last sentence of the original email. And I
am sorry about sending the email too fast. What I was trying to do is to make
a trivial statement: there's a mistake that needs to be fixed - let's do just
that and move on.

Cos

On Tue, Sep 29, 2015 at 03:00AM, Konstantin Boudnik wrote:
> Thank you you for confirming my point: there was a mistake and it needs to be
> corrected. End of story. But instead of simply fixing it and moving on, we are
> spending hours x 50 people on reading and writing long emails arguing about
> imaginary semantical differences.
> 
> There's no need to be emotional about who said what: I deserve the benefits of
> the doubt as well despite being a "former mentor of the project" whatever the
> hell it means. I am not dismissing the value of your contributions to this
> community: I welcome and appreciate your efforts! Neither have I targeted you
> nor put your on the stand - you did it to yourself. If you don't like
> something you think I addressed to you - send me a private email and explain
> that I was a jerk and hurt your feelings: no need to make a public display of
> potential nothingness. Would I choose to listen to it or not - is a separate
> matter altogether: I have the same right to not read or accept something that
> another person has wrote.
> 
> Let's move on. Best,
>   Cos
> 
> On Tue, Sep 29, 2015 at 01:16AM, Raul Kripalani wrote:
> > There has been no negligence, Cos! People are human and make mistakes. End
> > of the story.
> > 
> > Bringing such negative verbiage to the community helps in nothing.
> > Everybody is doing their best, I'd like to think so.
> > 
> > In fact, you have shifted the conversation away from the actual topic at
> > hand. So thanks.
> > 
> > A suggestion: "Benefit of the doubt" is a powerful practice and keeps us
> > away from errors in judgement.
> > 
> > With regards your list of questions, may I ask you to re-read your initial
> > message. Don't make me explain what's obvious, mate.
> > 
> > Cheers.
> > On 28 Sep 2015 23:41, "Konstantin Boudnik"  wrote:
> > 
> > > Hmm...
> > >
> > > Negligence, n. : the trait of neglecting responsibilities and lacking
> > > concern
> > > syn : omission, oversight
> > >
> > > Doesn't sound catastrophic in my vocabulary, really. Does this
> > > > case of negligence and needs to be addressed accordingly.
> > > translate to "should face a firing squad without a trial of his peers"?
> > > Have I anywhere pointed a finger at you or anyone else? Or attacked
> > > someone? Why are you all upset and defensive about it?
> > >
> > > Cos
> > >
> > > On September 28, 2015 7:39:51 AM PDT, Raul Kripalani 
> > > wrote:
> > > >Cos, your language seems too harsh for the situation.
> > > >
> > > >No one here is committing negligence. The explanation is simple: people
> > > >aren't perfect.
> > > >
> > > >Now, let's take a step back and see the big picture. Around 95% of the
> > > >commits in this project are by GridGain personnel (check git shortlog
> > > >-s
> > > >-n) who have spent months/years working on this codebase during their
> > > >daily
> > > >job. Their eyes are accustomed to this code style and naturally they'll
> > > >spot oddities in a twitch. It's obvious.
> > > >
> > > >For newer people, we don't even have checkstyle nor decent facilities
> > > >for
> > > >newer people to spot formatting issues quickly. Because, surprise! The
> > > >issues that Yakov spotted are simply of formatting. The code is
> > > >functional
> > > >and much better tested than other streamers and IP Finders. Other
> > > >streamers
> > > >have 1 test, this streamer has 9 unit tests! Look at the code.
> > > >Furthermore,
> > > >Yakov seems to have made a mistake reading the Git commit history.
> > > >There
> > > >were never WIP commits on master.
> > > >
> > > >So may I ask you to stop using catastrophic vocabulary. The situation
> > > >is
> > > >not catastrophic, it's simply improvable.
> > > >
> > > >Now, as an ASF member, I ask you to recognise that unaffiliated
> > > >volunteers
> > > >like me bring diversity to the project that's otherwise dominated by a
> > > >company. You should appreciate that – more so given that you're a
> > > >former
> > > >mentor. I do this for the fun, and attacks like yours take the fun out
> > > >of
> > > >it. Have a look again at this project's team composition and, for those
> > > >people not affiliated to GridGain, try to find when their last commit
> > > >was... Then you'll see what I mean.
> > > >
> > > >P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
> > > >Valentin will want to comment.
> > > >
> > > >Regards,
> > > >
> > > >*Raúl Kripalani*
> > > >PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
> > > >and
> > > >Messaging Engineer
> > > >http://about.me/raulkripalani |
> > > 

Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Raul Kripalani]

Cos, your language seems too harsh for the situation.

No one here is committing negligence. The explanation is simple: people
aren't perfect.

Now, let's take a step back and see the big picture. Around 95% of the
commits in this project are by GridGain personnel (check git shortlog -s
-n) who have spent months/years working on this codebase during their daily
job. Their eyes are accustomed to this code style and naturally they'll
spot oddities in a twitch. It's obvious.

For newer people, we don't even have checkstyle nor decent facilities for
newer people to spot formatting issues quickly. Because, surprise! The
issues that Yakov spotted are simply of formatting. The code is functional
and much better tested than other streamers and IP Finders. Other streamers
have 1 test, this streamer has 9 unit tests! Look at the code. Furthermore,
Yakov seems to have made a mistake reading the Git commit history. There
were never WIP commits on master.

So may I ask you to stop using catastrophic vocabulary. The situation is
not catastrophic, it's simply improvable.

Now, as an ASF member, I ask you to recognise that unaffiliated volunteers
like me bring diversity to the project that's otherwise dominated by a
company. You should appreciate that – more so given that you're a former
mentor. I do this for the fun, and attacks like yours take the fun out of
it. Have a look again at this project's team composition and, for those
people not affiliated to GridGain, try to find when their last commit
was... Then you'll see what I mean.

P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
Valentin will want to comment.

Regards,

*Raúl Kripalani*
PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data and
Messaging Engineer
http://about.me/raulkripalani | http://www.linkedin.com/in/raulkripalani
http://blog.raulkr.net | twitter: @raulvk

On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik  wrote:

> Are these official guidelines that are worked-out and communicated by
> community? Basically, were they made clear when the project went on the CTR
> model? I presume it is/was looking at the wikipage. Hence non-sticking
> to them is a case of negligence and needs to be addressed accordingly.
>
> I would also want to highlight the other side of such negligence: by
> dumping
> semi-baked code to the master one creates a burden for the rest of the
> community as the code degrades in quality, potentially breaks tests, style
> checks, etc. And someone else needs to deal with it to unblock her's future
> progress. And that's brings forward another point that Brane and I were
> making on a few occasions: in the CTR communities you need to invite in
> people
> with great deal of attention to how they work with others. Are they
> respecting
> others' time and effort? Are they good citizens of the community? And on,
> and
> on.
>
> Another purely technically matter: master isn't a trash can. Master should
> be
> close to releasable at any given point of time. WIP stuff doesn't belong to
> master, that's what the dev and integration branches are for.
>
> Cos
>
> On Mon, Sep 28, 2015 at 03:31PM, Yakov Zhdanov wrote:
> > Guys,
> >
> > I have just reviewed the code and have some comments. 1-2 are very
> serious
> > from my point of view.
> >
> > 1. Code is in master. Did anyone checked tests on TC? Moreover, are there
> > suites for those tests?
> > 2. It seems that work on streamer has been done directly in master. I see
> > WIP commits, but I think I should not. As agreed finished work should be
> > committed as a single set of changes.
> > 3. I see unused variable
> > - org.apache.ignite.stream.mqtt.MqttStreamer#cachedLogPrefix
> > 4. Unused import - import com.google.common.base.Joiner;
> > 5. Code and javadocs lines exceed 120 chars restriction.
> > 6. Plenty of javadocs issues - absence, multiline "inheritdoc", etc.
> > 7. Spacing is not correct - in ignite codebase logical blocks are
> separated
> > with blank line.
> > 8. There should always be a blank line at the end of each file.
> > 9. retrier vs retryier issue.
> >
> > Who is in charge for this code? Raul, Val? Can anyone fix my comments?
> >
> > I would also ask everyone (even committers) not to commit to master
> without
> > doing review with another committer.
> >
> > Here is the link to Ignite's coding guidelines -
> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines.
> Feel
> > free to suggest and discuss edits if anything does not seem valid to you.
> >
> > Thanks!
> >
> > --Yakov
>

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Dmitriy Setrakyan]

Cos,

I think Raul's point was that coding guidelines are not very clear. I think
Raul thought that he was following the coding guidelines. I don't think
"negligence" is a fair word to describe this.

In my view, we have a couple of omissions in the process on Wiki that need
to be spelled out clearly:

- semantic blocks should be described better in the coding guidelines
- we should clearly state that master should always be release-ready in the
lira process
- the best practice is to go through review in the ticket ignite-1234
branch, instead of in master.

I will try to fix it and send it here for review.

D.

On Tue, Sep 29, 2015 at 12:41 AM, Konstantin Boudnik  wrote:

> Hmm...
>
> Negligence, n. : the trait of neglecting responsibilities and lacking
> concern
> syn : omission, oversight
>
> Doesn't sound catastrophic in my vocabulary, really. Does this
> > case of negligence and needs to be addressed accordingly.
> translate to "should face a firing squad without a trial of his peers"?
> Have I anywhere pointed a finger at you or anyone else? Or attacked
> someone? Why are you all upset and defensive about it?
>
> Cos
>
> On September 28, 2015 7:39:51 AM PDT, Raul Kripalani 
> wrote:
> >Cos, your language seems too harsh for the situation.
> >
> >No one here is committing negligence. The explanation is simple: people
> >aren't perfect.
> >
> >Now, let's take a step back and see the big picture. Around 95% of the
> >commits in this project are by GridGain personnel (check git shortlog
> >-s
> >-n) who have spent months/years working on this codebase during their
> >daily
> >job. Their eyes are accustomed to this code style and naturally they'll
> >spot oddities in a twitch. It's obvious.
> >
> >For newer people, we don't even have checkstyle nor decent facilities
> >for
> >newer people to spot formatting issues quickly. Because, surprise! The
> >issues that Yakov spotted are simply of formatting. The code is
> >functional
> >and much better tested than other streamers and IP Finders. Other
> >streamers
> >have 1 test, this streamer has 9 unit tests! Look at the code.
> >Furthermore,
> >Yakov seems to have made a mistake reading the Git commit history.
> >There
> >were never WIP commits on master.
> >
> >So may I ask you to stop using catastrophic vocabulary. The situation
> >is
> >not catastrophic, it's simply improvable.
> >
> >Now, as an ASF member, I ask you to recognise that unaffiliated
> >volunteers
> >like me bring diversity to the project that's otherwise dominated by a
> >company. You should appreciate that – more so given that you're a
> >former
> >mentor. I do this for the fun, and attacks like yours take the fun out
> >of
> >it. Have a look again at this project's team composition and, for those
> >people not affiliated to GridGain, try to find when their last commit
> >was... Then you'll see what I mean.
> >
> >P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
> >Valentin will want to comment.
> >
> >Regards,
> >
> >*Raúl Kripalani*
> >PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
> >and
> >Messaging Engineer
> >http://about.me/raulkripalani |
> >http://www.linkedin.com/in/raulkripalani
> >http://blog.raulkr.net | twitter: @raulvk
> >
> >On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik 
> >wrote:
> >
> >> Are these official guidelines that are worked-out and communicated by
> >> community? Basically, were they made clear when the project went on
> >the CTR
> >> model? I presume it is/was looking at the wikipage. Hence
> >non-sticking
> >> to them is a case of negligence and needs to be addressed
> >accordingly.
> >>
> >> I would also want to highlight the other side of such negligence: by
> >> dumping
> >> semi-baked code to the master one creates a burden for the rest of
> >the
> >> community as the code degrades in quality, potentially breaks tests,
> >style
> >> checks, etc. And someone else needs to deal with it to unblock her's
> >future
> >> progress. And that's brings forward another point that Brane and I
> >were
> >> making on a few occasions: in the CTR communities you need to invite
> >in
> >> people
> >> with great deal of attention to how they work with others. Are they
> >> respecting
> >> others' time and effort? Are they good citizens of the community? And
> >on,
> >> and
> >> on.
> >>
> >> Another purely technically matter: master isn't a trash can. Master
> >should
> >> be
> >> close to releasable at any given point of time. WIP stuff doesn't
> >belong to
> >> master, that's what the dev and integration branches are for.
> >>
> >> Cos
> >>
> >> On Mon, Sep 28, 2015 at 03:31PM, Yakov Zhdanov wrote:
> >> > Guys,
> >> >
> >> > I have just reviewed the code and have some comments. 1-2 are very
> >> serious
> >> > from my point of view.
> >> >
> >> > 1. Code is in master. Did anyone checked tests on TC? Moreover, are
> >there
> >> > suites for those tests?
> >> > 2. It seems that 

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Konstantin Boudnik]

Hmm...

Negligence, n. : the trait of neglecting responsibilities and lacking concern
syn : omission, oversight

Doesn't sound catastrophic in my vocabulary, really. Does this
> case of negligence and needs to be addressed accordingly.
translate to "should face a firing squad without a trial of his peers"?
Have I anywhere pointed a finger at you or anyone else? Or attacked someone? 
Why are you all upset and defensive about it?

Cos

On September 28, 2015 7:39:51 AM PDT, Raul Kripalani  wrote:
>Cos, your language seems too harsh for the situation.
>
>No one here is committing negligence. The explanation is simple: people
>aren't perfect.
>
>Now, let's take a step back and see the big picture. Around 95% of the
>commits in this project are by GridGain personnel (check git shortlog
>-s
>-n) who have spent months/years working on this codebase during their
>daily
>job. Their eyes are accustomed to this code style and naturally they'll
>spot oddities in a twitch. It's obvious.
>
>For newer people, we don't even have checkstyle nor decent facilities
>for
>newer people to spot formatting issues quickly. Because, surprise! The
>issues that Yakov spotted are simply of formatting. The code is
>functional
>and much better tested than other streamers and IP Finders. Other
>streamers
>have 1 test, this streamer has 9 unit tests! Look at the code.
>Furthermore,
>Yakov seems to have made a mistake reading the Git commit history.
>There
>were never WIP commits on master.
>
>So may I ask you to stop using catastrophic vocabulary. The situation
>is
>not catastrophic, it's simply improvable.
>
>Now, as an ASF member, I ask you to recognise that unaffiliated
>volunteers
>like me bring diversity to the project that's otherwise dominated by a
>company. You should appreciate that – more so given that you're a
>former
>mentor. I do this for the fun, and attacks like yours take the fun out
>of
>it. Have a look again at this project's team composition and, for those
>people not affiliated to GridGain, try to find when their last commit
>was... Then you'll see what I mean.
>
>P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
>Valentin will want to comment.
>
>Regards,
>
>*Raúl Kripalani*
>PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
>and
>Messaging Engineer
>http://about.me/raulkripalani |
>http://www.linkedin.com/in/raulkripalani
>http://blog.raulkr.net | twitter: @raulvk
>
>On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik 
>wrote:
>
>> Are these official guidelines that are worked-out and communicated by
>> community? Basically, were they made clear when the project went on
>the CTR
>> model? I presume it is/was looking at the wikipage. Hence
>non-sticking
>> to them is a case of negligence and needs to be addressed
>accordingly.
>>
>> I would also want to highlight the other side of such negligence: by
>> dumping
>> semi-baked code to the master one creates a burden for the rest of
>the
>> community as the code degrades in quality, potentially breaks tests,
>style
>> checks, etc. And someone else needs to deal with it to unblock her's
>future
>> progress. And that's brings forward another point that Brane and I
>were
>> making on a few occasions: in the CTR communities you need to invite
>in
>> people
>> with great deal of attention to how they work with others. Are they
>> respecting
>> others' time and effort? Are they good citizens of the community? And
>on,
>> and
>> on.
>>
>> Another purely technically matter: master isn't a trash can. Master
>should
>> be
>> close to releasable at any given point of time. WIP stuff doesn't
>belong to
>> master, that's what the dev and integration branches are for.
>>
>> Cos
>>
>> On Mon, Sep 28, 2015 at 03:31PM, Yakov Zhdanov wrote:
>> > Guys,
>> >
>> > I have just reviewed the code and have some comments. 1-2 are very
>> serious
>> > from my point of view.
>> >
>> > 1. Code is in master. Did anyone checked tests on TC? Moreover, are
>there
>> > suites for those tests?
>> > 2. It seems that work on streamer has been done directly in master.
>I see
>> > WIP commits, but I think I should not. As agreed finished work
>should be
>> > committed as a single set of changes.
>> > 3. I see unused variable
>> > - org.apache.ignite.stream.mqtt.MqttStreamer#cachedLogPrefix
>> > 4. Unused import - import com.google.common.base.Joiner;
>> > 5. Code and javadocs lines exceed 120 chars restriction.
>> > 6. Plenty of javadocs issues - absence, multiline "inheritdoc",
>etc.
>> > 7. Spacing is not correct - in ignite codebase logical blocks are
>> separated
>> > with blank line.
>> > 8. There should always be a blank line at the end of each file.
>> > 9. retrier vs retryier issue.
>> >
>> > Who is in charge for this code? Raul, Val? Can anyone fix my
>comments?
>> >
>> > I would also ask everyone (even committers) not to commit to master
>> without
>> > doing review with another committer.
>> >
>> > Here is the link to Ignite's 

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Raul Kripalani]

There has been no negligence, Cos! People are human and make mistakes. End
of the story.

Bringing such negative verbiage to the community helps in nothing.
Everybody is doing their best, I'd like to think so.

In fact, you have shifted the conversation away from the actual topic at
hand. So thanks.

A suggestion: "Benefit of the doubt" is a powerful practice and keeps us
away from errors in judgement.

With regards your list of questions, may I ask you to re-read your initial
message. Don't make me explain what's obvious, mate.

Cheers.
On 28 Sep 2015 23:41, "Konstantin Boudnik"  wrote:

> Hmm...
>
> Negligence, n. : the trait of neglecting responsibilities and lacking
> concern
> syn : omission, oversight
>
> Doesn't sound catastrophic in my vocabulary, really. Does this
> > case of negligence and needs to be addressed accordingly.
> translate to "should face a firing squad without a trial of his peers"?
> Have I anywhere pointed a finger at you or anyone else? Or attacked
> someone? Why are you all upset and defensive about it?
>
> Cos
>
> On September 28, 2015 7:39:51 AM PDT, Raul Kripalani 
> wrote:
> >Cos, your language seems too harsh for the situation.
> >
> >No one here is committing negligence. The explanation is simple: people
> >aren't perfect.
> >
> >Now, let's take a step back and see the big picture. Around 95% of the
> >commits in this project are by GridGain personnel (check git shortlog
> >-s
> >-n) who have spent months/years working on this codebase during their
> >daily
> >job. Their eyes are accustomed to this code style and naturally they'll
> >spot oddities in a twitch. It's obvious.
> >
> >For newer people, we don't even have checkstyle nor decent facilities
> >for
> >newer people to spot formatting issues quickly. Because, surprise! The
> >issues that Yakov spotted are simply of formatting. The code is
> >functional
> >and much better tested than other streamers and IP Finders. Other
> >streamers
> >have 1 test, this streamer has 9 unit tests! Look at the code.
> >Furthermore,
> >Yakov seems to have made a mistake reading the Git commit history.
> >There
> >were never WIP commits on master.
> >
> >So may I ask you to stop using catastrophic vocabulary. The situation
> >is
> >not catastrophic, it's simply improvable.
> >
> >Now, as an ASF member, I ask you to recognise that unaffiliated
> >volunteers
> >like me bring diversity to the project that's otherwise dominated by a
> >company. You should appreciate that – more so given that you're a
> >former
> >mentor. I do this for the fun, and attacks like yours take the fun out
> >of
> >it. Have a look again at this project's team composition and, for those
> >people not affiliated to GridGain, try to find when their last commit
> >was... Then you'll see what I mean.
> >
> >P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
> >Valentin will want to comment.
> >
> >Regards,
> >
> >*Raúl Kripalani*
> >PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
> >and
> >Messaging Engineer
> >http://about.me/raulkripalani |
> >http://www.linkedin.com/in/raulkripalani
> >http://blog.raulkr.net | twitter: @raulvk
> >
> >On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik 
> >wrote:
> >
> >> Are these official guidelines that are worked-out and communicated by
> >> community? Basically, were they made clear when the project went on
> >the CTR
> >> model? I presume it is/was looking at the wikipage. Hence
> >non-sticking
> >> to them is a case of negligence and needs to be addressed
> >accordingly.
> >>
> >> I would also want to highlight the other side of such negligence: by
> >> dumping
> >> semi-baked code to the master one creates a burden for the rest of
> >the
> >> community as the code degrades in quality, potentially breaks tests,
> >style
> >> checks, etc. And someone else needs to deal with it to unblock her's
> >future
> >> progress. And that's brings forward another point that Brane and I
> >were
> >> making on a few occasions: in the CTR communities you need to invite
> >in
> >> people
> >> with great deal of attention to how they work with others. Are they
> >> respecting
> >> others' time and effort? Are they good citizens of the community? And
> >on,
> >> and
> >> on.
> >>
> >> Another purely technically matter: master isn't a trash can. Master
> >should
> >> be
> >> close to releasable at any given point of time. WIP stuff doesn't
> >belong to
> >> master, that's what the dev and integration branches are for.
> >>
> >> Cos
> >>
> >> On Mon, Sep 28, 2015 at 03:31PM, Yakov Zhdanov wrote:
> >> > Guys,
> >> >
> >> > I have just reviewed the code and have some comments. 1-2 are very
> >> serious
> >> > from my point of view.
> >> >
> >> > 1. Code is in master. Did anyone checked tests on TC? Moreover, are
> >there
> >> > suites for those tests?
> >> > 2. It seems that work on streamer has been done directly in master.
> >I see
> >> > WIP commits, but I 

Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer

[Dmitriy Setrakyan]

One more point about empty lines. I have reviewed our empty line policy and
I don't think I can do a better job describing it. The explanation is
pretty accurate.

However, the main problem with our empty line policy is that, although the
resulting code looks very neat, the policy is just too complex. It would be
nice if someone with correct code style settings could create a correct
auto-format configuration for IDEA and Eclipse. Any volunteers?

D.

On Tue, Sep 29, 2015 at 2:44 AM, Dmitriy Setrakyan 
wrote:

>
>
> On Tue, Sep 29, 2015 at 12:49 AM, Dmitriy Setrakyan <
> dsetrak...@gridgain.com> wrote:
>
>> Cos,
>>
>> I think Raul's point was that coding guidelines are not very clear. I
>> think Raul thought that he was following the coding guidelines. I don't
>> think "negligence" is a fair word to describe this.
>>
>> In my view, we have a couple of omissions in the process on Wiki that
>> need to be spelled out clearly:
>>
>> - semantic blocks should be described better in the coding guidelines
>> - we should clearly state that master should always be release-ready in
>> the lira process
>> - the best practice is to go through review in the ticket ignite-1234
>> branch, instead of in master.
>>
>> I will try to fix it and send it here for review.
>>
>
> Guys, I think we have a very confusing Wiki. We have GIT Process page and
> Jira Process pages. Both of them describe some part of the same process. I
> think they should be combined into 1 page with GIT and JIRA sections. We
> can call this page GIT and JIRA Process.
>
> Also, I could not find any place where we mention that contributors may
> create a GitHub pull request. Since we accept pull requests, that section
> definitely needs to be added.
>
> Thoughts?
>
>
>>
>> D.
>>
>> On Tue, Sep 29, 2015 at 12:41 AM, Konstantin Boudnik 
>> wrote:
>>
>>> Hmm...
>>>
>>> Negligence, n. : the trait of neglecting responsibilities and lacking
>>> concern
>>> syn : omission, oversight
>>>
>>> Doesn't sound catastrophic in my vocabulary, really. Does this
>>> > case of negligence and needs to be addressed accordingly.
>>> translate to "should face a firing squad without a trial of his peers"?
>>> Have I anywhere pointed a finger at you or anyone else? Or attacked
>>> someone? Why are you all upset and defensive about it?
>>>
>>> Cos
>>>
>>> On September 28, 2015 7:39:51 AM PDT, Raul Kripalani 
>>> wrote:
>>> >Cos, your language seems too harsh for the situation.
>>> >
>>> >No one here is committing negligence. The explanation is simple: people
>>> >aren't perfect.
>>> >
>>> >Now, let's take a step back and see the big picture. Around 95% of the
>>> >commits in this project are by GridGain personnel (check git shortlog
>>> >-s
>>> >-n) who have spent months/years working on this codebase during their
>>> >daily
>>> >job. Their eyes are accustomed to this code style and naturally they'll
>>> >spot oddities in a twitch. It's obvious.
>>> >
>>> >For newer people, we don't even have checkstyle nor decent facilities
>>> >for
>>> >newer people to spot formatting issues quickly. Because, surprise! The
>>> >issues that Yakov spotted are simply of formatting. The code is
>>> >functional
>>> >and much better tested than other streamers and IP Finders. Other
>>> >streamers
>>> >have 1 test, this streamer has 9 unit tests! Look at the code.
>>> >Furthermore,
>>> >Yakov seems to have made a mistake reading the Git commit history.
>>> >There
>>> >were never WIP commits on master.
>>> >
>>> >So may I ask you to stop using catastrophic vocabulary. The situation
>>> >is
>>> >not catastrophic, it's simply improvable.
>>> >
>>> >Now, as an ASF member, I ask you to recognise that unaffiliated
>>> >volunteers
>>> >like me bring diversity to the project that's otherwise dominated by a
>>> >company. You should appreciate that – more so given that you're a
>>> >former
>>> >mentor. I do this for the fun, and attacks like yours take the fun out
>>> >of
>>> >it. Have a look again at this project's team composition and, for those
>>> >people not affiliated to GridGain, try to find when their last commit
>>> >was... Then you'll see what I mean.
>>> >
>>> >P.S.: I did not merge the ZK IP Finder myself and I'm assuming that
>>> >Valentin will want to comment.
>>> >
>>> >Regards,
>>> >
>>> >*Raúl Kripalani*
>>> >PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
>>> >and
>>> >Messaging Engineer
>>> >http://about.me/raulkripalani |
>>> >http://www.linkedin.com/in/raulkripalani
>>> >http://blog.raulkr.net | twitter: @raulvk
>>> >
>>> >On Mon, Sep 28, 2015 at 1:53 PM, Konstantin Boudnik 
>>> >wrote:
>>> >
>>> >> Are these official guidelines that are worked-out and communicated by
>>> >> community? Basically, were they made clear when the project went on
>>> >the CTR
>>> >> model? I presume it is/was looking at the wikipage. Hence
>>> >non-sticking
>>> >> to them is a case of negligence and needs to be