Open Stack

Oh, looks like we got a bit of a race condition in messages. I hope you
don't mind.


On Wed, Aug 6, 2014 at 11:00 PM, Ben Nemec <openstack at nemebean.com> wrote:

> On 08/06/2014 01:42 PM, Yuriy Taraday wrote:
> > On Wed, Aug 6, 2014 at 6:20 PM, Ben Nemec <openstack at nemebean.com>
> wrote:
> >
> >> On 08/06/2014 03:35 AM, Yuriy Taraday wrote:
> >>> 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.
> >>
> >> Right, I understand that may not be the intent, but it's almost
> >> certainly going to be the end result.  You can't control how people are
> >> going to use this feature, and history suggests if it can be abused, it
> >> will be.
> >>
> >
> > Can you please outline the abuse scenario that isn't present nowadays?
> > People upload huge changes and are encouraged to split them during
> review.
> > The same will happen within proposed workflow. More experienced
> developers
> > split their change into a set of change requests. The very same will
> happen
> > within proposed workflow.
>
> There will be a documented option in git-review that automatically
> squashes all commits.  People _will_ use that incorrectly because from a
> submitter perspective it's easier to deal with one review than multiple,
> but from a reviewer perspective it's exactly the opposite.
>

It won't be documented as such. It will include "use with care" and "years
of Git experience: 3+" stickers. Autosquashing will never be mentioned
there. Only a detailed explanation of how to work with it and (probably)
how it works. No rogue dev will get through it without getting the true
understanding.

By the way, currently git-review suggests to squash your outstanding
commits but there is no overwhelming flow of overly huge change requests,
is there?

>>> On Wed, Aug 6, 2014 at 12:03 PM, Martin Geisler <martin at geisler.net>
> >> wrote:
> >>>
> >>>> Ben Nemec <openstack at nemebean.com> writes:
> >>>>
> >>>>> On 08/05/2014 03:14 PM, Yuriy Taraday wrote:
> >>>>>>
> >>>>>> When you're developing some big change you'll end up with trying
> >>>>>> dozens of different approaches and make thousands of mistakes. For
> >>>>>> reviewers this is just unnecessary noise (commit title "Scratch my
> >>>>>> last CR, that was bullshit") while for you it's a precious history
> >>>>>> that can provide basis for future research or bug-hunting.
> >>>>>
> >>>>> So basically keeping a record of how not to do it?  I get that, but I
> >>>>> think I'm more onboard with the suggestion of sticking those dead end
> >>>>> changes into a separate branch.  There's no particular reason to keep
> >>>>> them on your working branch anyway since they'll never merge to
> master.
> >>>>>  They're basically unnecessary conflicts waiting to happen.
> >>>>
> >>>> Yeah, I would never keep broken or unfinished commits around like
> this.
> >>>> In my opinion (as a core Mercurial developer), the best workflow is to
> >>>> work on a feature and make small and large commits as you go along.
> When
> >>>> the feature works, you begin squashing/splitting the commits to make
> >>>> them into logical pieces, if they aren't already in good shape. You
> then
> >>>> submit the branch for review and iterate on it until it is accepted.
> >>>>
> >>>
> >>> 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.
> >>> 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.
> >>>
> >>>
> >>>> As a reviewer, it cannot be stressed enough how much small, atomic,
> >>>> commits help. Squashing things together into large commits make
> reviews
> >>>> very tricky and removes the possibility of me accepting a later commit
> >>>> while still discussing or rejecting earlier commits (cherry-picking).
> >>>>
> >>>
> >>> 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.
> >>> 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.
> >>>
> >>>> FWIW, I have had long-lived patch series, and I don't really see what
> >>>>> is so difficult about running git rebase master. Other than
> conflicts,
> >>>>> of course, which are going to be an issue with any long-running
> change
> >>>>> no matter how it's submitted. There isn't a ton of git magic
> involved.
> >>>>
> >>>> I agree. The conflicts you talk about are intrinsic to the parallel
> >>>> development. Doing a rebase is equivalent to doing a series of merges,
> >>>> so if rebase gives you conflicts, you can be near certain that a plain
> >>>> merge would give you conflicts too. The same applies other way around.
> >>>>
> >>>
> >>> 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.
> >>
> >> This is a separate issue though.  Editing a commit in the middle of a
> >> series doesn't have to be done at the same time as a rebase to master.
> >>
> >
> > No, this will be done with a separate interactive rebase or that detached
> > HEAD and reflog dance. I don't see this as smth clearer than doing proper
> > commits in a separate branches.
>
> You keep mentioning detached HEAD and reflog.  I have never had to deal
> with either when doing a rebase, so I think there's a disconnect here.
> The only time I see a detached HEAD is when I check out a change from
> Gerrit (and I immediately stick it in a local branch, so it's a
> transitive state), and the reflog is basically a safety net for when I
> horribly botch something, not a standard tool that I use on a daily basis.
>

It usually takes some time for me to build trust in utility that does a lot
of different things at once while I need only one small piece of that. So I
usually do smth like:
$ git checkout HEAD~2
$ vim
$ git commit
$ git checkout mybranch
$ git rebase --onto HEAD@{1} HEAD~2
instead of almost the same workflow with interactive rebase.

> In fact, not having a bunch of small broken commits that can't be
> >> submitted individually in your history makes it _easier_ to deal with
> >> follow-up changes.  Then you know that the unit tests pass on every
> >> commit, so you can work on it in isolation without constantly having to
> >> rebase through your entire commit history.  This workflow seems to
> >> encourage the painful rebases you're trying to avoid.
> >>
> >
> > No, this workflow encourage using merges instead of rebases. You don't
> need
> > to rebase anything.
>
> /shrug
>
> The end result of both is the same (your change applied to master), but
> the merge version leaves your local repo a mess with a merge commit that
> might be overwriting things from your previous commits, but you won't
> know just by looking at one in isolation.
>

Just two commands:
$ git diff HEAD~
will show you what had changed in your branch with this merge;
$ git diff HEAD^2
will show your new diff against master.
You won't be able to do both after rebase without scratching some commit ID
in your notebook (or a temporary branch).

-- 

Kind regards, Yuriy.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140807/3e51671a/attachment.html>