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. 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... > This reduces the cost of code review for the reviewer - which is where > we have a bottleneck. >> It is very clear that the current process is neither streamlined nor is it >> painless, so change is clearly necessary. > The process is streamlined and painless, when both parties agree to > make it easy. See the past few pull-requests for examples which show > that it works well. And paid staff should stick with the agreed process. But we have agreement on this now from Xor, so that's fine. > The code review of fcp-rewrite was exhausting, because (a) the > pull-request was huge, and (b) the coder got defensive and long-winded > instead of accepting the review where he agreed or giving short, > friendly explanations where he thought that it was mistaken - and > accept that code which does not pass the review cannot be merged. > > (a) will happen from time to time. By requiring refactoring of history > for huge pull-requests and generally making it as easy as reasonable > to review we can deal with it (though what’s reasonable might differ - > in the end it’s the reviewers who have to decide that, because they > are the bottleneck). > > (b) however is not about the tools. It is about the question what > behavior we as community can expect from paid developers, and how to > organize our community in a way which minimizes the friction from very > different time budgets. > > Best wishes, > Arne
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Devl mailing list Devl@freenetproject.org https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
