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 Mon, 28 Sep 2015 22:41:56 GMT
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
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message