On Sun, Mar 22, 2015 at 1:36 AM, Florent Daigniere < [email protected]> wrote:
> On Sun, 2015-03-22 at 00:42 +0000, Matthew Toseland wrote: > > On 21/03/15 20:49, Ian Clarke wrote: > > > On Sat, Mar 21, 2015 at 1:58 PM, Matthew Toseland < > [email protected]> > > > wrote: > > >> Most of the above boils down to "review the diff, not the history". > That > > >> is probably sensible. > > > That's part of it, but also that a branch should be created for each > > > bugfix/feature, which ideally should be as small a unit of work as > possible > > > (that can be merged without breaking stuff). > > It only is if the diff is of reasonable size... which it is most of the > time, *except* when it comes from paid devs. > Then that needs to change. With a feature/bugfix-per-branch approach the diffs should be reviewable in just a few minutes. If they aren't, then the coder is probably stuffing multiple features/bugfixes into a single branch and that's contrary to the guidelines I've outlined. Whatever happened in the past, what matters is what we do going forward. > They code in their cave for months, drop a 100kloc diff doing way more > than a single feature/bugfix onto the maintainer and expect it to be > merged; I'm sure that refactoring is good but not when it's forked off > for 6month... This is the problem we had recently with both toad's and > xor's code. We're talking about dozens of features and bugfixes AND > refactoring. > Then that needs to change going forward. > Whether there's a single change per commit/branch/pull request doesn't > matter as long as one of them is enforced. The way Github's code review tools are designed, you definitely want it to be per branch (which will have a 1-1 relationship with a pull request), not per commit. > Until now the base unit was > supposed to be "one change per commit"; I'm all for changing it to > something else but that won't solve the problem unless it's enforced > strictly. > You can't impose processes on people, they need to agree to them or it won't work. That being said, I don't know why any reasonable person wouldn't agree to what I've outlined. It's a tried and tested approach. Ian. _______________________________________________ Devl mailing list [email protected] https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
