On Tue, Mar 27, 2012 at 6:06 AM, Tim Starling tstarling@wikimedia.org wrote:
On 27/03/12 19:49, Roan Kattouw wrote:
On Mon, Mar 26, 2012 at 9:50 PM, Tim Starling tstarling@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