On 04/03/2015 02:19 PM, Matthew Toseland wrote: > On 01/04/15 21:56, Arne Babenhauserheide wrote: >> Am Dienstag, 31. März 2015, 22:29:24 schrieb Ian: >>> On Mon, Mar 30, 2015 at 3:16 PM, Arne Babenhauserheide <arne_...@web.de> >>> wrote: >>>> It’s work from paid contributors for which >>>> we need structures which reduce the cost of code-review compared to >>>> what you propose >>> I haven't heard of any proposal other than my own that would reduce the >>> cost of code-review. What specifically has been proposed that would reduce >>> the cost of code review, and why specifically would it reduce the cost of >>> code-review? >> Requiring contributors to refactor large pull-requests into >> semantically meaningful commits. > Or into separate pull requests. Which is what Ian suggested. > > Sometimes a change will be big enough that it needs to be treated as a > series of changes of manageable size. > > However these changes will still be dependent on each other. For example > my work this summer should have been broken up into something like: > > Add infrastructure > Add splitfile storage backend > Add splitfile storage tests > Integrate splitfile storage > Implement back compatibility > Delete remnants of old code > Improve APIs since we've had to change them anyway > > (This is an oversimplification, of course...) > > These could be separate pull requests, but they have to be treated as a > series, because after "integrate splitfile storage", we lose backwards > compatibility. > > Ideally we'd like to avoid such situations entirely but when dealing > with persistence, legacy code, and refactoring APIs, we can't always > achieve this, and sometimes it's not desirable (i.e. we want to get all > the API changes in simultaneously - they can be separate pull requests > but should go in for the same build). > > The point is, the two approaches are functionally equivalent: > - A series of largish squashed commits. > - A series of pull requests. > > IMHO the former is marginally preferable, because a series of pull > requests is a slightly awkward beast. But I don't think it matters much. > > Pick one, continue. Committers (including the release manager) should > require one or the other.
We all seem to agree that small pull requests that can be reviewed in a few minutes is the ideal. I propose the contributing guidelines include something like this: > ## Pull request scope > > A pull request is easiest to review when it is small enough take only > a few minutes. If your pull request is much larger than this, we > might ask that you split it over a series of pull requests. I don't see why we need to require exactly one technique. What about keeping the part about squashing with an intro like this: > If you'd rather keep a single pull request, please squash into > high-level commits: If there are objections to including this suggestion even at this level I will drop it so that we can proceed. > But that's not the real issue here. > > The fundamental disagreement here is *whether we should have advance > code review at all*. Unfortunately this appears to be a case of > conflicting goals... I didn't take that as serious proposals that no one review code at all, rather an example of how we really do need to figure out a way to review effectively.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Devl mailing list Devl@freenetproject.org https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
