ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Boudnik <...@apache.org>
Subject Re: Fwd: Coding Guidelines: zookeeper IP finder and mqtt streamer
Date Tue, 29 Sep 2015 11:05:44 GMT
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" <cos@apache.org> 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 <raulk@apache.org>
> > > 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 <cos@apache.org>
> > > >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
> > > >>
> > >

Mime
View raw message