ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Boudnik <...@apache.org>
Subject Re: Coding Guidelines: zookeeper IP finder and mqtt streamer
Date Mon, 28 Sep 2015 12:53:27 GMT
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