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

[Gaëtan Lehmann]

Le 19 oct. 10 à 19:54, Marcus D. Hanwell a écrit :

> 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.

The classes touched by this patch were already in the ITK repository.
I've squashed as much changes as I can to avoid the bug fixing patches  
of my previous non yet integrated patches and make the number of  
patches not to big.

>
>>> 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.


Maybe this tool is not the right one for this kind of work: everything  
here goes in Review and should have been reviewed by the IJ reviewers  
first.
It may have been better to commit most of my changes directly in ITK,  
and to have put only the patches not strictly from the IJ contribution  
in gerrit. That would have reduced the list to

   http://review.source.kitware.com/#change,181
   http://review.source.kitware.com/#change,186
   http://review.source.kitware.com/#change,197
   http://review.source.kitware.com/#change,199
   http://review.source.kitware.com/#change,198
   http://review.source.kitware.com/#change,208
   http://review.source.kitware.com/#change,207
   http://review.source.kitware.com/#change,206

Gaëtan

-- 
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr  http://www.itk.org
http://www.mandriva.org  http://www.bepo.fr

-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 203 bytes
Desc: Ceci est une signature ?lectronique PGP
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20101019/03f67270/attachment.pgp>