<div dir="ltr">This is purely anecdotal, but on the ecosystem side for a lot of our projects I've tried to psuedo-enforce the "one commit", or really, a change/fix/feature per commit. Thereby allowing me to cherrypick for patch releases to stable (or revert a commit) with confidence and without a lot of hunting for the right grouping.<div><br></div><div>With the advent of squashing in github I've dropped this push and use this unless the author has already done the logical grouping of commits, in which case I'll merge them myself, out of github, to avoid merge messages but retain their grouping (and potentially modify commit messages, to make it easier to identify the PR number and the bug number it fixes).<div><br></div><div>I don't think the Juju core project can just carte blanche squash every pull request, but I do think it's up to the code authors to put an effort in to squashing/rewriting/managing their commits prior to submittion to make the code's history more observable and manageable overtime. Much in the same way you would document or comment blocks of code, commits are a window into what this patch does, if you want to keep your history, for reference, branching is cheap in git and you absolutely can.</div><div><br></div><div>Happy to share more of the latter mentioned workflow for those interested, but otherwise just some 2¢</div><div><br></div><div>Marco</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jun 16, 2016 at 10:12 AM John Meinel <<a href="mailto:john@arbash-meinel.com">john@arbash-meinel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Note that if github is squashing the commits when it lands into Master, I <i>believe</i> that this breaks the ancestry with your local branch. So it isn't a matter of "the history just isn't present in the master branch", but "it looks like a completely independent commit revision, and you have no obvious way to associate it with the branch that you have at all".<div><br></div><div>It may be that git adds information to the commit ('this commit is a rollup of hash deadbeef") in which case the git tool could look it up.</div><div><br></div><div>I don't know the github UI around this. If I do "git merge --squash" then it leaves me with an uncommitted tree with the file contents updated, and then I can do "git commit -m new-content"</div><div><br></div><div>And then if I try to do:</div><div>$ git branch -d test-branch</div><div>error: The branch 'test-branch' is not fully merged.</div><div>If you are sure you want to delete it, run 'git branch -D test-branch'</div><div><br></div><div>Which indicates to me that it intentionally forgets everything about all of your commits, which mean <b>you</b> need to know when it got merged so that you can prune your branches, because the tool isn't going to track what has and hasn't been merged.</div><div><br></div><div>(I don't know about other people, but because of the delays of waiting for reviews and merge bot bouncing things, it can take a while for something to actually land. I often have branches that sit for a while, and it is easy for me to not be 100% sure if that quick bugfix I did last week actually made it through to master, and having 'git branch -d ' as a short hand was quite useful.)</div><div><br></div><div>Note that if we are going to go with "only 1 commit for each thing landed", then I do think that using github's squash feature is probably better than rebasing your branches. Because if we just rebase your branch, then you end up with 2 revisions that represent your commit (the one you proposed, and the merge revision), vs just having the "revision of master that represents your changes rebased onto master". We could 'fast forward when possible' but that just means there is a window where sometimes you rebased your branch and it landed fast enough to be only 1 commit, vs someone else landed a change just before you and now you have a merge commit. I would like us to be consistent.</div><div><br></div><div>For people who do want to throw away history with a rebase, what's your feeling on whether there should be a merge commit (the change as I proposed it) separate from the change-as-it-landed-on-master. I mean, if you're getting rid of the history, the the change-as-I-proposed-it (and personally tested it) doesn't really matter, right? And the bot tests the change-as-it-landed.</div><div><br>John</div><div>=:-></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 16, 2016 at 4:20 PM, Ian Booth <span dir="ltr"><<a href="mailto:ian.booth@canonical.com" target="_blank">ian.booth@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><br>
<br>
On 16/06/16 19:04, David Cheney wrote:<br>
> Counter suggestion: the bot refuses to accept PR's that contain more<br>
> than one commit, then it's up to the submitter to prepare it in any<br>
> way that they feel appropriate.<br>
><br>
<br>
</span>Please no. I do not want to be forced to alter my local history.<br>
<br>
I was hopeful that having the landing bot / github squash commits would satisfy<br>
those people what did not want to use git log --first-parent to present a<br>
sanitised view of commits, but allow me to retain the history in my branches<br>
locally so I could look back on what I did and when and how (if needed).<br>
<br>
If the default github behaviour is not sufficient, perhaps we can add some<br>
tooling in the merge bot to do the squashing prior to merging.<br>
<div><div><br>
<br>
> On Thu, Jun 16, 2016 at 6:44 PM, roger peppe <<a href="mailto:roger.peppe@canonical.com" target="_blank">roger.peppe@canonical.com</a>> wrote:<br>
>> Squashed commits are nice, but there's something worth watching<br>
>> out for: currently the merge commit is committed with the text<br>
>> that's in the github PR, but when a squashed commit is made, this<br>
>> text is ignored and only the text in the actual proposed commit ends up<br>
>> in the history. This surprised me (I often edit the PR description<br>
>> as the review continues) so worth being aware of, I think.<br>
>><br>
>>   cheers,<br>
>>     rog.<br>
>><br>
>> On 16 June 2016 at 02:12, Menno Smits <<a href="mailto:menno.smits@canonical.com" target="_blank">menno.smits@canonical.com</a>> wrote:<br>
>>> Hi everyone,<br>
>>><br>
>>> Following on from the recent thread about commit squashing and commit<br>
>>> message quality, the idea of automatically squashing commit at merge time<br>
>>> has been raised. The idea is that the merge bot would automatically squash<br>
>>> commits for a pull request into a single commit, using the PR description as<br>
>>> the commit message.<br>
>>><br>
>>> With this in place, developers can commit locally using any approach they<br>
>>> prefer. The smaller commits they make as they work won't be part of the<br>
>>> history the team interacts with in master.<br>
>>><br>
>>> When using autosquashing the quality of pull request descriptions should get<br>
>>> even more scrutiny during reviews. The quality of PR descriptions is already<br>
>>> important as they are used for merge commits but with autosquashing in place<br>
>>> they will be the *only* commit message.<br>
>>><br>
>>> Autosquashing can be achieved technically by either having the merge bot do<br>
>>> the squashing itself, or by taking advantage of Github's feature to do this<br>
>>> (currently in preview mode):<br>
>>><br>
>>> <a href="https://developer.github.com/changes/2016-04-01-squash-api-preview/" rel="noreferrer" target="_blank">https://developer.github.com/changes/2016-04-01-squash-api-preview/</a><br>
>>><br>
>>> We need to ensure that the squashed commits are attributed to the correct<br>
>>> author (i.e. not jujubot). I'm not sure what we do with pull requests which<br>
>>> contain work from multiple authors. There doesn't seem to be an established<br>
>>> approach for this.<br>
>>><br>
>>> Thoughts?<br>
>>><br>
>>> - Menno<br>
>>><br>
>>><br>
>>><br>
>>><br>
>>> --<br>
>>> Juju-dev mailing list<br>
>>> <a href="mailto:Juju-dev@lists.ubuntu.com" target="_blank">Juju-dev@lists.ubuntu.com</a><br>
>>> Modify settings or unsubscribe at:<br>
>>> <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
>>><br>
>><br>
>> --<br>
>> Juju-dev mailing list<br>
>> <a href="mailto:Juju-dev@lists.ubuntu.com" target="_blank">Juju-dev@lists.ubuntu.com</a><br>
>> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
><br>
<br>
--<br>
Juju-dev mailing list<br>
<a href="mailto:Juju-dev@lists.ubuntu.com" target="_blank">Juju-dev@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
</div></div></blockquote></div><br></div>
--<br>
Juju-dev mailing list<br>
<a href="mailto:Juju-dev@lists.ubuntu.com" target="_blank">Juju-dev@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
</blockquote></div>