On 03/22/2015 05:32 PM, Matthew Toseland wrote: > Here's a narrow question which we should agree an answer to: > > Is it OK to merge a pull request only having reviewed the diff? > (I am thinking of relatively small changes here, i.e. typical pull requests) > > In the past I have generally reviewed every commit, in case there are > problems (e.g. backdoors!) hidden that might be hit when somebody does a > git bisection searching for a bug. > > However, this may be overly paranoid. > > It can be avoided completely: If a pull request is more than one commit, > and the commits aren't helpful to the review process, then the person > doing the merge can either squash them into one commit, or request that > the author does so. Hence we get a clean, reviewed history, without > having to review every commit as bugs are added and then removed. In > practice, for straightforward pull requests, one commit will generally > be better anyway.
My inclination is to merge the unit of review. If the diff can be easily described in a single commit summary, the pull request is squashed and merged as a single commit. If individual commits are needed to break up a change into easily-described and reviewed pieces, they are merged without squashing. (Or with squashing code review results into the high-level separation; see "git commit --squash/--fixup".) There is a section about it in the proposed contributing standards: https://github.com/freenet/fred/pull/338/files#diff-6a3371457528722a734f3c51d9238c13R80 I'd like to get to a community consensus - thoughts on this?
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Devl mailing list Devl@freenetproject.org https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
