On Tue, Jul 28, 2015 at 01:09AM, Branko Čibej wrote: > On 27.07.2015 18:29, Sergi Vladykin wrote: > > Guys, > > > > I would say Ignite is quite a big and quite complex project. > > This has absolutely nothing to do with trusting developers. FWIW, > Subversion is also very complex, I'll dare say that its object model and > working copy semantics are more convoluted than anything you'll find in > Ignite. We make do with CTR quite well. > > > > Also we have > > really tough requirements for performance, stability, backward > > compatibility, etc... > > Write them down. Make sure new contributors understand them before > offering them commit access. That's a prerequisite for growing an open > community anyway. > > > Having said that it is really easy to break something > > even with minor change like switching type of collection to another one. > > So? Stuff like that should be documented in the code, if it isn't no > amount of prior review will help, since how do you know that the > reviewer happens to remember such details a few years from now? > > > And I personally feel safer when my code has been reviewed before I merge > > it to master. > > So ask for a review if you think you need it. I never said *all* reviews > should be post-commit: I said that the default mode should be CTR and > it's up to the developer to ask for more eyes on their code. > > > Also when thousand lines change set is merged and there are > > conflicting change sets merged after, it is quite hard to rollback this > > first change if it was wrong. And we have conflicting changes all the time. > > Apparently you need to learn to use the version control tool you > selected, or adjust your workflows for the failings of said tool. Or > change the tool, who knows, you might find that's a good move. :) > > Regardless of which tool you use, conflicts should *ALWAYS* be resolved > on the development branch, not on the mainline: the workflow should be: > > 1. create branch from mainline > 2. make changes on branch > 3. merge mainline to branch and resolve conflicts (repeat as needed) > 4. run tests on branch code > 5. merge branch to mainline > > Skipping step 3 (and 4) is going to be a constant pain regardless of > which tool you use. Also for trivial changes it makes no sense at all to > even create a branch; just fix it on the mainline, your version history > will be a lot easier to follow. > > > My opinion is that correct trade off for us now is having slower but more > > predictable development. RTC approach here definitely fits better. > > RTC has an additional problem that it can (and often does; look at one > of the most famous ASF projects if you don't believe me) create tensions > in the community. It makes sense to use it for maintenance branches, but > not for new development.
Needless to say, I agree with everything just said (except for the VCS change proposition, which I found to be outright crazy, of course ;) And to pile more on top: I've experienced the consequences of the very situation, described in the very last paragraph. The RTC lead to a totally nuts situation in Hadoop, where fellas who wanted to reject a small patch, not based on the technical merits of the fix, had came up with an excuse that the reviewer (a full committer) didn't have enough expertise to review the code in question. The silliness was quickly put down by the sane community members, but you see where I am going with it... In other words: if you don't trust a committer to make the changes in the VCS system - that committer shouldn't be having the commit-bit in the first place. Intricacies of roll-backs aren't an excuse and most often than not could be fixed by changing the process hygiene. CTR attitude leads, in general, to a higher quality of the contribution because ppl are trying to avoid public scolding, inevitable in case of forced-reverts. With RTC the blame would be shared between the author and the reviewer, which make situation more bearable (falsely, of course). And guess what - reviewers are humans and make mistakes all the time, but that was already pointed out. Cos
