On Aug 6, 2014 7:21 AM, "Ben Nemec" <openst...@nemebean.com> wrote: > > On 08/05/2014 03:14 PM, Yuriy Taraday wrote: > > On Tue, Aug 5, 2014 at 10:48 PM, Ben Nemec <openst...@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@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
