[Insight-developers] gerrit: Ouch...my mouse clicking finger is sore ; )

[Marcus D. Hanwell]

On Tue, Oct 19, 2010 at 1:25 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
> What is the magic patch that will include all of the patches?

http://review.source.kitware.com/#change,209
>
> On Tue, Oct 19, 2010 at 1:19 PM, Hans Johnson <hans-johnson at uiowa.edu> wrote:
>> So here’s my experience with gerrit on the 33 patches as part of the
>> labelmap topic branch.
>>
>> I like the separate patches, and think that from a development point of view
>> this is definitely the best approach.

Personally, I would not have made a single commit for each
class...Gerrit or no Gerrit. I would have also squashed a few of the
commits such as,

http://review.source.kitware.com/#change,200

This is something that I would edit in my local history before publish.

>> It was annoying to click so many times to do a review that was so closely
>> related for every item.
>> Gerrit was mind-numbingly PAINFUL to use in to approve these.  It took
>> almost 10 minutes to click through each of the review stages pushing the
>> same buttons, and pasting the same comment.

Like I said, such large topic are likely to be painful either way. The
web interface is certainly not optimized for topic branches either.
Would the command line Gerrit tools have been useful?

http://gerrit.googlecode.com/svn/documentation/2.1.5/cmd-review.html

$ ssh review.source.kitware.com gerrit review --project ITK --message
\"This comment entered from the command line.\" --verified 1
--code-review 1 eb675d9870bc94ebe8bd8d87968ee36cfe5e7ff9
>>
>> I know I’m complaining a lot here, but it is because I really want gerrit to
>> succeed.  It took me more than an hour to review this topic branch, verify
>> that it works, and record my approval of the patches.
>>
I think it all depends on whether the commits ever needed to be
reviewed independently. If they did not then I am not sure they needed
their own commits, but that is up for debate. If it is just the number
of clicks you had to make in order to approve a topic branch, then I
wonder if an 'Approve topic branch' solution would be most
appropriate?

The problem is if you ask someone to squash all of their commits into
one for review, ask for a change to be made before approval do they
then need to go back to their topic, edit its history, re-squash it,
then add back in the Change-Id and when it is finally merged have what
is merged bear no resemblance to what Gerrit say/approved?

Gerrit has a fairly well developed command line interface if you would
like a functionality to approve a topic branch. I guess when the patch
set is so big it is unlikely people will have time to fully review the
changes. The merge is certainly a simple operation, merging in the tip
of the topic will merge all other commits.

I would tend to bring something this big in as a full topic, using git
to view the diff while it is compiling. So perhaps a way of approving
a topic in its entirety, and this is something we probably want to
work towards in the web interface too.

Marcus