Another reason I do not like reviewing topics that modify the same file in multiple changes...<div><br></div><div>I routinely cherry-pick topics rather than checkout topics. This way, when I build the topic, only the topic's files and dependencies are rebuilt. My builds go much faster. Also when I'm done with my topic review, I</div>
<div>git checkout master</div><div><br></div><div>and once again rebuilding goes much faster. If the topic modifies the same file in multiple changes, I often get conflicts with the cherry picking.</div><div><br></div><div>
I realize this only affects me because of the workflow I've been using for over a year for both ITK and VTK.</div><div><br></div><div>Bill<br><br><div class="gmail_quote">On Tue, Oct 9, 2012 at 10:33 AM, Marcus D. Hanwell <span dir="ltr"><<a href="mailto:marcus.hanwell@kitware.com" target="_blank">marcus.hanwell@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I pointed out in my reply that this is an outstanding feature request<br>
we would like to add to Gerrit to make this easier. You can download<br>
the topic and view the diff locally, but we haven't had time to work<br>
on this yet (totally agree that is a failing in Gerrit).<br>
<br>
The point about viewing history stands when trying to figure out where<br>
a bug came from, but maybe I am the only one who does that? I don't<br>
think it adds a lot of value seeing the minutia of what you did, and<br>
so tend to use git commit --amend to tack on minor changes to the<br>
previous commit, I prefer that practice but wouldn't try to force it<br>
on all (and wasn't trying to in my reply).<br>
<span class="HOEnZb"><font color="#888888"><br>
Marcus<br>
</font></span><div class="im HOEnZb"><br>
On Tue, Oct 9, 2012 at 9:47 AM, Moreland, Kenneth <<a href="mailto:kmorel@sandia.gov">kmorel@sandia.gov</a>> wrote:<br>
</div><div class="HOEnZb"><div class="h5">> I may be alone in this opinion, but I attribute this entire email thread<br>
> as caused more by a failing of Gerrit rather than a failing of developers.<br>
>  It really shouldn't matter whether a topic branch has one commit or a<br>
> thousand.  If Gerrit truly respects reviews on topic branches, then it<br>
> should be able to provide a holistic diff between the head of the topic<br>
> branch and the commit where it branched from the master branch.  Then<br>
> there wouldn't be any difference review-wise between having one monolithic<br>
> commit or lots of small fixes.<br>
><br>
> -Ken<br>
><br>
><br>
><br>
> On 10/9/12 7:04 AM, "Marcus D. Hanwell" <<a href="mailto:marcus.hanwell@kitware.com">marcus.hanwell@kitware.com</a>> wrote:<br>
><br>
>>In the days of CVS you would work on changes locally (or I would)<br>
>>until they were ready. When it looked good you would push before 12pm<br>
>>EST, watch the continuous and make commits to fix what might have been<br>
>>broken.<br>
>><br>
>>When working on changes I often have a topic where I get things<br>
>>working, push to Gerrit to get CDash@Home results/feedback and make<br>
>>use of git add files/I/changed and git commit --amend to add my<br>
>>changes to the last commit in the topic.<br>
>><br>
>>It makes topics harder to review if there are lots of small fixes etc<br>
>>as individual commits. This is a matter of preference, and due to<br>
>>supporting topics we can at least group all of these changes. Once we<br>
>>have rebased and have some time an outstanding feature request is a<br>
>>diff of the entire topic (in addition to individual commits in the<br>
>>topic).<br>
>><br>
>>I personally think appending commits to topics that apply suggested<br>
>>fixes is fine, although I tend to just use git commit --amend before<br>
>>pushing. If there are lots of tiny commits it can be harder to review,<br>
>>and looking at history when trying to figure out what change<br>
>>introduced a bug can be more challenging/noisier.<br>
>><br>
>>I am not sure what you mean by atomic commits, but when we switched to<br>
>>Git the advice was functional commits, i.e. each commit should stand<br>
>>alone as a logical change. This doesn't mean they have to be tiny, and<br>
>>I don't remember seeing advice advocating committing each step as you<br>
>>develop.<br>
>><br>
>>The process is evolving, and several of us have worked to improve<br>
>>things such as distributed workflows, testing of proposed patches,<br>
>>code review, improved testing, build system modernization.<br>
>><br>
>>Marcus<br>
>><br>
>>On Sun, Oct 7, 2012 at 4:42 PM, Philippe Pebay<br>
>><<a href="mailto:philippe.pebay@kitware.com">philippe.pebay@kitware.com</a>> wrote:<br>
>>> Hello Bill<br>
>>><br>
>>> Thanks for your note. I am an atomic-type :) This is a leftover from CVS<br>
>>> times.<br>
>>> In fact I think that VTK still officially recommends atomic commits.<br>
>>>Isn't<br>
>>> it so?<br>
>>><br>
>>> Let me see what I can do with this one. But don't worry if you don't<br>
>>>have<br>
>>> time, we'll find a way.<br>
>>><br>
>>> Thanks!<br>
>>> Philippe<br>
>>><br>
>>><br>
>>> On Sun, Oct 7, 2012 at 10:, Bill Lorensen <<a href="mailto:bill.lorensen@gmail.com">bill.lorensen@gmail.com</a>><br>
>>>wrote:<br>
>>>><br>
>>>> Philippe,<br>
>>>><br>
>>>> First, I think we all appreciate gerrit as a review process. And as a<br>
>>>> gerrit patch contributor, I appreciate how much work it takes to<br>
>>>>create,<br>
>>>> test and prepare patches for gerrit.<br>
>>>><br>
>>>> But, as a gerrit reviewer, I find it difficult to do a good job<br>
>>>>reviewing<br>
>>>> a topic that has almost 20 changes. I've seen several people doing<br>
>>>>this.<br>
>>>><br>
>>>> I'm at the other extreme. My topics typically have one change.<br>
>>>><br>
>>>> There is probably a good intermediate approach.<br>
>>>><br>
>>>> Question: In this topic, <a href="http://review.source.kitware.com/#/t/1415/" target="_blank">http://review.source.kitware.com/#/t/1415/</a> ,<br>
>>>> copuld the number of changes been reduced?<br>
>>>><br>
>>>> Bill<br>
>>>><br>
>>>><br>
>>>><br>
>>><br>
>>><br>
>>><br>
>>> --<br>
>>> Philippe Pébay, PhD<br>
>>> Director of Visualization and High Performance Computing /<br>
>>> Directeur de la Visualisation et du Calcul Haute Performance<br>
>>> Kitware SAS<br>
>>> 26 rue Louis Guérin, 69100 Villeurbanne, France<br>
>>> <a href="tel:%2B33%20%280%29%206.83.61.55.70" value="+33683615570">+33 (0) 6.83.61.55.70</a> / 4.37.45.04.15<br>
>>> <a href="http://www.kitware.fr" target="_blank">http://www.kitware.fr</a><br>
>>><br>
>>> _______________________________________________<br>
>>> Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
>>><br>
>>> Visit other Kitware open-source projects at<br>
>>> <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>>><br>
>>> Follow this link to subscribe/unsubscribe:<br>
>>> <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
>>><br>
>>><br>
>>_______________________________________________<br>
>>Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
>><br>
>>Visit other Kitware open-source projects at<br>
>><a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>><br>
>>Follow this link to subscribe/unsubscribe:<br>
>><a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
>><br>
>><br>
><br>
><br>
_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>Unpaid intern in BillsBasement at noware dot com<br><br>
</div>