On Nov 2, 2012, at 2:47 PM, Randall Leeds <[email protected]> wrote:
> On Fri, Nov 2, 2012 at 6:50 AM, Robert Newson <[email protected]> wrote: > >> Sure, and it couldn't hurt to clarify how we should use git in these >> circumstances. The principal rule is that each commit should be >> self-contained and, further, that it should at least potentially be worth >> reading in the future. A bug in a branch, found during a review and then >> fixed, does not to be preserved and should be squashed into a single >> commit. I would expect most bugfix branches to have a single commit, and >> most feature branches to have several (where each is incrementally >> introducing the feature, or refactoring to make way for it). And, of >> course, we need the prohibition on merge commits lifted so that can >> preserve that history and record the point at which it joined master. >> > > Unfortunately, once the Pull Request is opened, it's hard to modify it. > With review, it's easy to add more commits, but I do not think asking > contributors to force push their PR branch is a good idea. > > From the Pull Request UI on GitHub, the Files Changed tab is essentially > the squashed view, while the Commits tab shows the individual commits. I > tend to review the squashed view. > > From here, one possible best flow is for the *committer* to squash the > branch in the merge and add the appropriate "close #" or "fix #" to the > commit message, so that the PR is automatically closed. Of course, that > loses the authorship information. So one could do a "git merge --squash" > followed by a "git commit --author=". > > But I wonder if squashing just isn't the wrong thing to do anyway. If we > allow merge commits, instead, it becomes possible to use "git log --merges" > to get the nice, squashy history, but we don't have to muck around with > anything at all. I've even heard people make the case for *always* using > --no-ff on every merge. > > "git log --first-parent" might be even more useful, since (if I'm not > mistaken) it would show fast-forward pushes committers have made straight > to the repo, but only show the merge commits from pull requests and feature > merges. > > I think if we want to play nice with pull requests, we may want to > re-evaluate our no-merges policy. That all makes a ton of sense to me, but I'm sure I've forgotten the very good reasons that we had for not using merge commits in the beginning. Adam
