> #1 Yakov, how exactly are you reading the Git history? WIP commits were > never on master, they were in a feature branch which was merged into master > on this commit:
You can check the history of the streamer. I see more than one entry. Probably, that was mistake on merge - commits were not squashed. Please don't take it personally. I am talking to everyone - please squash commits when merge to master! This is described here - https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-Forcommitters > #2 About TC tests, please see this discussion: > http://apache-ignite-developers.2346864.n4.nabble.com/Maybe-review-my-first-commits-td3370.html Again, you committed to master without checking TC :) No? https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-Forcontributors - you omitted "check TC step". I think if TC not working, nothing should be committed to master unless community agrees that commit is safe. As a workaround we still have ability to check the patch internally (if Ignite TC not working) and we could do it for you. Btw, TC should be recovered (at least partially) - http://204.14.53.153/ > (a) the Coding Guidelines are not meticulous enough for the level of > auditing the community is doing; I get the impression that subjective bias > is playing a part here, e.g. the "logical blocks" comment -- can you > provide an objective definition accepted by industry? please check code around and you can easily catch what is meant here. Take a look, for example, at GridNearAtomicUpdateFuture > (d) for the people who are so concerned with this level of detail, would > you consider writing a checkstyle definition? It'll provide an objective > basis, and we can plug it into the build and make it fail. That'll be the > end of the story ;-) I doubt if I will ever do that. Let's wait for someone else :-), till that moment we will be doing peer reviews =) > By the way, I'm sure you are reviewing outdated code because Joiner *is* used. I had warning in IDE. It could be problem on my side. Now I see it is used. > Will reply to your individual formatting comments later.. Please don't forget about - unused members - empty javadocs > #4 While we are on this, can we please discuss stuff like this: > https://imgur.com/991Aa4X. Why doesn't the team use the --rebase option > when pulling? Also, we need to remember that branch names disappear when > the branch is deleted, therefore the commits lose their context (ticket, > feature, etc.) if the message does not carry it. You can see how the commit > log looks to another person in the screenshot. Should we care about this if we squash on pushing to master? Thanks! --Yakov
