<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">I'd like to stress this to everyone: I DO NOT propose squashing together commits that should belong to separate change requests. I DO NOT propose to upload all your changes at once. I DO propose letting developers to keep local history of all iterations they have with a change request. The history that absolutely doesn't matter to anyone but this developer.</div>

<div class="gmail_quote"><br></div><div class="gmail_quote">On Wed, Aug 6, 2014 at 12:03 PM, Martin Geisler <span dir="ltr"><<a href="mailto:martin@geisler.net" target="_blank">martin@geisler.net</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">Ben Nemec <<a href="mailto:openstack@nemebean.com">openstack@nemebean.com</a>> writes:<br>
<br>
> On 08/05/2014 03:14 PM, Yuriy Taraday wrote:<br>
>><br>
</div><div class="">>> When you're developing some big change you'll end up with trying<br>
>> dozens of different approaches and make thousands of mistakes. For<br>
>> reviewers this is just unnecessary noise (commit title "Scratch my<br>
>> last CR, that was bullshit") while for you it's a precious history<br>
>> that can provide basis for future research or bug-hunting.<br>
><br>
> So basically keeping a record of how not to do it?  I get that, but I<br>
> think I'm more onboard with the suggestion of sticking those dead end<br>
> changes into a separate branch.  There's no particular reason to keep<br>
> them on your working branch anyway since they'll never merge to master.<br>
>  They're basically unnecessary conflicts waiting to happen.<br>
<br>
</div>Yeah, I would never keep broken or unfinished commits around like this.<br>
In my opinion (as a core Mercurial developer), the best workflow is to<br>
work on a feature and make small and large commits as you go along. When<br>
the feature works, you begin squashing/splitting the commits to make<br>
them into logical pieces, if they aren't already in good shape. You then<br>
submit the branch for review and iterate on it until it is accepted.<br></blockquote><div><br></div><div>Absolutely true. And it's mostly the same workflow that happens in OpenStack: you do your cool feature, you carve meaningful small self-contained pieces out of it, you submit series of change requests.</div>

<div>And nothing in my proposal conflicts with it. It just provides a way to make developer's side of this simpler (which is the intent of git-review, isn't it?) while not changing external artifacts of one's work: the same change requests, with the same granularity.</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
As a reviewer, it cannot be stressed enough how much small, atomic,<br>
commits help. Squashing things together into large commits make reviews<br>
very tricky and removes the possibility of me accepting a later commit<br>
while still discussing or rejecting earlier commits (cherry-picking).<br></blockquote><div><br></div><div>That's true, too. But please don't think I'm proposing to squash everything together and push 10k-loc patches. I hate that, too. I'm proposing to let developer use one's tools (Git) in a simpler way.</div>

<div>And the simpler way (for some of us) would be to have one local branch for every change request, not one branch for the whole series. Switching between branches is very well supported by Git and doesn't require extra thinking. Jumping around in detached HEAD state and editing commits during rebase requires remembering all those small details.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> FWIW, I have had long-lived patch series, and I don't really see what<br>
> is so difficult about running git rebase master. Other than conflicts,<br>
> of course, which are going to be an issue with any long-running change<br>
> no matter how it's submitted. There isn't a ton of git magic involved.<br>
<br>
</div>I agree. The conflicts you talk about are intrinsic to the parallel<br>
development. Doing a rebase is equivalent to doing a series of merges,<br>
so if rebase gives you conflicts, you can be near certain that a plain<br>
merge would give you conflicts too. The same applies other way around.<br></blockquote><div><br></div><div>You disregard other issues that can happen with patch series. You might need something more that rebase. You might need to fix something. You might need to focus on the one commit in the middle and do huge bunch of changes in it alone. And I propose to just allow developer to keep track of what's one been doing instead of forcing one to remember all of this.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> So as you may have guessed by now, I'm opposed to adding this to<br>
> git-review. I think it's going to encourage bad committer behavior<br>
> (monolithic commits) and doesn't address a use case I find compelling<br>
> enough to offset that concern.<br>
<br>
</div>I don't understand why this would even be in the domain of git-review. A<br>
submitter can do the "puff magic" stuff himself using basic Git commands<br>
before he submits the collapsed commit.<br></blockquote></div><br clear="all"><div>Isn't it the domain of git-review - "puff magic"? You can upload your changes with 'git push HEAD:refs/for/master', you can do all your rebasing by yourself, but somehow we ended up with this tool that simplifies common tasks related to uploading changes to Gerrit.</div>

<div>And (at least for some) such change would simplify their day-to-day workflow with regards to uploading changes to Gerrit.</div><div><br></div>-- <br><br><div>Kind regards, Yuriy.</div>
</div></div>