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
