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 <ra...@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 <c...@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 >>
