[vtk-developers] Gerrit topics with large numbers of changes

Moreland, Kenneth kmorel at sandia.gov
Tue Oct 9 09:47:41 EDT 2012


I may be alone in this opinion, but I attribute this entire email thread
as caused more by a failing of Gerrit rather than a failing of developers.
 It really shouldn't matter whether a topic branch has one commit or a
thousand.  If Gerrit truly respects reviews on topic branches, then it
should be able to provide a holistic diff between the head of the topic
branch and the commit where it branched from the master branch.  Then
there wouldn't be any difference review-wise between having one monolithic
commit or lots of small fixes.

-Ken



On 10/9/12 7:04 AM, "Marcus D. Hanwell" <marcus.hanwell at kitware.com> wrote:

>In the days of CVS you would work on changes locally (or I would)
>until they were ready. When it looked good you would push before 12pm
>EST, watch the continuous and make commits to fix what might have been
>broken.
>
>When working on changes I often have a topic where I get things
>working, push to Gerrit to get CDash at Home results/feedback and make
>use of git add files/I/changed and git commit --amend to add my
>changes to the last commit in the topic.
>
>It makes topics harder to review if there are lots of small fixes etc
>as individual commits. This is a matter of preference, and due to
>supporting topics we can at least group all of these changes. Once we
>have rebased and have some time an outstanding feature request is a
>diff of the entire topic (in addition to individual commits in the
>topic).
>
>I personally think appending commits to topics that apply suggested
>fixes is fine, although I tend to just use git commit --amend before
>pushing. If there are lots of tiny commits it can be harder to review,
>and looking at history when trying to figure out what change
>introduced a bug can be more challenging/noisier.
>
>I am not sure what you mean by atomic commits, but when we switched to
>Git the advice was functional commits, i.e. each commit should stand
>alone as a logical change. This doesn't mean they have to be tiny, and
>I don't remember seeing advice advocating committing each step as you
>develop.
>
>The process is evolving, and several of us have worked to improve
>things such as distributed workflows, testing of proposed patches,
>code review, improved testing, build system modernization.
>
>Marcus
>
>On Sun, Oct 7, 2012 at 4:42 PM, Philippe Pebay
><philippe.pebay at kitware.com> wrote:
>> Hello Bill
>>
>> Thanks for your note. I am an atomic-type :) This is a leftover from CVS
>> times.
>> In fact I think that VTK still officially recommends atomic commits.
>>Isn't
>> it so?
>>
>> Let me see what I can do with this one. But don't worry if you don't
>>have
>> time, we'll find a way.
>>
>> Thanks!
>> Philippe
>>
>>
>> On Sun, Oct 7, 2012 at 10:, Bill Lorensen <bill.lorensen at gmail.com>
>>wrote:
>>>
>>> Philippe,
>>>
>>> First, I think we all appreciate gerrit as a review process. And as a
>>> gerrit patch contributor, I appreciate how much work it takes to
>>>create,
>>> test and prepare patches for gerrit.
>>>
>>> But, as a gerrit reviewer, I find it difficult to do a good job
>>>reviewing
>>> a topic that has almost 20 changes. I've seen several people doing
>>>this.
>>>
>>> I'm at the other extreme. My topics typically have one change.
>>>
>>> There is probably a good intermediate approach.
>>>
>>> Question: In this topic, http://review.source.kitware.com/#/t/1415/ ,
>>> copuld the number of changes been reduced?
>>>
>>> Bill
>>>
>>>
>>>
>>
>>
>>
>> --
>> Philippe Pébay, PhD
>> Director of Visualization and High Performance Computing /
>> Directeur de la Visualisation et du Calcul Haute Performance
>> Kitware SAS
>> 26 rue Louis Guérin, 69100 Villeurbanne, France
>> +33 (0) 6.83.61.55.70 / 4.37.45.04.15
>> http://www.kitware.fr
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>
>>
>_______________________________________________
>Powered by www.kitware.com
>
>Visit other Kitware open-source projects at
>http://www.kitware.com/opensource/opensource.html
>
>Follow this link to subscribe/unsubscribe:
>http://www.vtk.org/mailman/listinfo/vtk-developers
>
>





More information about the vtk-developers mailing list