On Tue, Mar 27, 2012 at 6:06 AM, Tim Starling <tstarl...@wikimedia.org> wrote:
> On 27/03/12 19:49, Roan Kattouw wrote:
>> On Mon, Mar 26, 2012 at 9:50 PM, Tim Starling <tstarl...@wikimedia.org> 
>> wrote:
>>> For commits with lots of files, Gerrit's diff interface is too broken
>>> to be useful. It does not provide a compact overview of the change
>>> which is essential for effective review.
>>>
>>> Luckily, there are alternatives, specifically local git clients and
>>> gitweb. However, these don't work when git's change model is broken by
>>> the use of git commit --amend.
>>>
>> They do; it just wasn't obvious to you how to do it, but that doesn't
>> mean it can't be done.
>>
>> $ git fetch https://gerrit.wikimedia.org/r/p/analytics/udp-filters
>> refs/changes/22/3222/3 && git branch foo FETCH_HEAD
>> $ git fetch https://gerrit.wikimedia.org/r/p/analytics/udp-filters
>> refs/changes/22/3222/4 && git branch bar FETCH_HEAD
>> $ git diff foo..bar
>>
>> The two 'git fetch' commands (or at least the part before the &&) can
>> be taken from the change page in Gerrit.
>
> It doesn't work, I'm afraid. Because of the implicit rebase on push,
> usually subsequent changesets have a different parent. So when you
> diff between the two branches, you get all of the intervening commits
> which were merged to the master.
>
> Examples from today:
>
> https://gerrit.wikimedia.org/r/#change,3367
> Patchsets 1 and 2 have different parents.
>
> https://gerrit.wikimedia.org/r/#change,3363
> Patchsets 1, 2 and 3 have different parents.
>
> It's possible to get a diff between them, and I did, but it's tedious.
> I figure we should pick a workflow that doesn't waste the reviewer's
> time quite so much.
>

The problem here is the implicit rebase. As long as the review
backlog isn't long and/or people aren't submitting conflicting
changes, rebasing amended changes against master creates
more harm than good.

For amending commits, you should use `git review -R` so you
don't rebase the change (again) against master (see for example
[0], difference between patch 2 and 3). I've updated the docs[1],
but they are, briefly:

git review -d 123
(make changes)
git commit -a --amend
git review -R

If you're not using git-review and have been using the alias, your
amended patchsets have not been creating this problem.

-Chad

[0] https://gerrit.wikimedia.org/r/#change,4020
[1] https://www.mediawiki.org/wiki/Git/Workflow#Amend_your_change

_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to