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

Tue Oct 9 10:58:55 EDT 2012

Another reason I do not like reviewing topics that modify the same file in
multiple changes...

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
git checkout master

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.

I realize this only affects me because of the workflow I've been using for
over a year for both ITK and VTK.

Bill

On Tue, Oct 9, 2012 at 10:33 AM, Marcus D. Hanwell <
marcus.hanwell at kitware.com> wrote:

> I pointed out in my reply that this is an outstanding feature request
> we would like to add to Gerrit to make this easier. You can download
> the topic and view the diff locally, but we haven't had time to work
> on this yet (totally agree that is a failing in Gerrit).
>
> The point about viewing history stands when trying to figure out where
> a bug came from, but maybe I am the only one who does that? I don't
> think it adds a lot of value seeing the minutia of what you did, and
> so tend to use git commit --amend to tack on minor changes to the
> previous commit, I prefer that practice but wouldn't try to force it
> on all (and wasn't trying to in my reply).
>
> Marcus
>
> On Tue, Oct 9, 2012 at 9:47 AM, Moreland, Kenneth <kmorel at sandia.gov>
> wrote:
> > 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
> >>
> >>
> >
> >
> _______________________________________________
> 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
>
>

-- 
Unpaid intern in BillsBasement at noware dot com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20121009/81bb18b8/attachment.html>