Open Stack

On Aug 6, 2014 7:21 AM, "Ben Nemec" <openstack at nemebean.com> wrote:
>
> On 08/05/2014 03:14 PM, Yuriy Taraday wrote:
> > On Tue, Aug 5, 2014 at 10:48 PM, Ben Nemec <openstack at nemebean.com>
wrote:
> >
> >> On 08/05/2014 10:51 AM, ZZelle wrote:
> >>> Hi,
> >>>
> >>>
> >>> I like the idea  ... with complex change, it could useful for the
> >>> understanding to split it into smaller changes during development.
> >>
> >> I don't understand this.  If it's a complex change that you need
> >> multiple commits to keep track of locally, why wouldn't reviewers want
> >> the same thing?  Squashing a bunch of commits together solely so you
> >> have one review for Gerrit isn't a good thing.  Is it just the warning
> >> message that git-review prints when you try to push multiple commits
> >> that is the problem here?
> >
> >
> > 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.
>
> >
> > Merges are one of the strong sides of Git itself (and keeping them very
> > easy is one of the founding principles behind it). With current
workflow we
> > don't use them at all. master went too far forward? You have to do
rebase
> > and screw all your local history and most likely squash everything
anyway
> > because you don't want to fix commits with known bugs in them. With
> > proposed feature you can just do merge once and let 'git review' add
some
> > magic without ever hurting your code.
>
> How do rebases screw up your local history?  All your commits are still
> there after a rebase, they just have a different parent.  I also don't
> see how rebases are all that much worse than merges.  If there are no
> conflicts, rebases are trivial.  If there are conflicts, you'd have to
> resolve them either way.
>
> I also reiterate my point about not keeping broken commits on your
> working branch.  You know at some point they're going to get
> accidentally submitted. :-)
>
> As far as letting git review do magic, how is that better than "git
> rebase once and no magic required"?  You deal with the conflicts and
> you're good to go.  And if someone asks you to split a commit, you can
> do it.  With this proposal you can't, because anything but squashing
> into one commit is going to be a nightmare (which might be my biggest
> argument against this).
>
> >
> > And speaking about breaking down of change requests don't forget support
> > for change requests chains that this feature would lead to. How to you
deal
> > with 5 consecutive change request that are up on review for half a year?
> > The only way I could suggest to my colleague at a time was "Erm... Learn
> > Git and dance with rebases, detached heads and reflogs!" My proposal
might
> > take care of that too.
> >
>
> How does this relate to commit series?  Squashing all the commits into
> one isn't a solution to any of the problems with those (if it were, we
> could do that today :-).
>
> 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.
>
> So as you may have guessed by now, I'm opposed to adding this to
> git-review.  I think it's going to encourage bad committer behavior
> (monolithic commits) and doesn't address a use case I find compelling
> enough to offset that concern.

+1

>
> /wall-o-text
>
> -Ben
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140806/2faf2d82/attachment.html>