On Fri, Nov 2, 2012 at 6:14 PM, Adam Kocoloski <[email protected]> wrote:
> 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 The only one I can recall is that we needed a way to go back to SVN if infra deemed the git experiment a failure. That meant linear history.
