On Tue, Dec 30, 2014 at 11:24 AM, Jeremy Stanley <fu...@yuggoth.org> wrote: > On 2014-12-30 12:31:35 -0500 (-0500), David Kranz wrote: > [...] >> 1. This is really a UI issue, and one that is experienced by many. >> What is desired is an option to look at different revisions of the >> patch that show only what the author actually changed, unless >> there was a conflict. > > I'm not sure it's entirely a UI issue. It runs deeper. There simply > isn't enough metadata in Git to separate intentional edits from > edits made to solve merge conflicts. Using merge commits instead of > rebases mostly solves this particular problem but at the expense of > introducing all sorts of new ones. A rebase-oriented workflow makes > it easier for merge conflicts to be resolved along the way, instead > of potentially nullifying valuable review effort at the very end > when it comes time to approve the change and it's no longer relevant > to the target branch.
Jeremy is correct here. I've dreamed about how to enhance git to support this sort of thing more formally but it isn't an easy problem and wouldn't help us in the short term anyway. To overcome this, I hacked out a script [1] which rebases older patch sets to the same parent as the most current patch set to help me compare across rebases. I've found it very handy in certain situations. I can see how conflicts were handled as well as what other changes were made outside the scope of merge conflict resolution. I use it by downloading the latest patch set with "git review -d XXXXX" and then I compare to a previous patch set (NN) by supplying that patch set number on the command line. I once had dreams of adding this capability to gerrit but I found the gerrit development learning curve to be a bit steep for the time I had. > There is a potential work-around, though it currently involves some > manual effort (not sure whether it would be sane to automate as a > feature of git-review). When you notice your change conflicts and > will need a rebase, first reset and stash your change, then reset > --hard to the previous patchset already in Gerrit, then rebase that > and push it (solving the merge conflicts if any), then pop your > stashed edits (solving any subsequent merge conflicts) and finally > push that as yet another patchset. This separates the rebase from > your intentional modifications though at the cost of rather a lot of > extra work. > > Alternatively you could push your edits with git review -R and > _then_ follow up with another patchset rebasing on the target branch > and resolving the merge conflicts. Possibly slightly easier? I'm a strong proponent of splitting rebases (with merge conflict resolution) from other manual changes. This is a help to reviewers. If someone tells me that a patch set is a pure rebase to resolve conflicts then I can "review" it by repeating the rebase myself to see if I get the same answer. Both suggestions above are good ones. Which one you use is a matter of preference IMO. I personally prefer the latter (push with -R and then resolve conflicts) because it is easier on me. >> 2. Using -R is dangerous unless you really know what you are >> doing. The doc string makes it sound like an innocuous way to help >> reviewers. > > Not necessarily dangerous, but it does allow you to push changes > which are just going to flat fail all jobs because they can't be > merged to the target branch to get tested. I agree there is no danger. As I've stated in my other post, I have *always* used it for two years and have seen no danger. I have come to accept the failing jobs as a regular and welcome part of my work flow. If these failing jobs are taking a lot of resources then we need some redesign in infrastructure to fail them more quickly and cheaply so that resources can be spared from having to test patch sets which are in conflict. Carl [1] http://paste.openstack.org/show/155614/ _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
