[vtk-developers] VTK code review / testing / integration workflow

Ben Boeckel ben.boeckel at kitware.com
Mon Aug 25 11:02:17 EDT 2014


On Mon, Aug 25, 2014 at 10:29:27 -0400, David Cole wrote:
> Perhaps, just perhaps, avoiding branch dependencies in the first place 
> is the proper solution to such a problem, rather than requiring your 
> review tool to handle it.

This usually arises from wide-sweeping changes which uncover other,
unrelated bugs in the process. For example, my performance changes for
CMake uncovered lots of little things which were a nightmare to review
as a single codedrop, but were untestable (in terms of finding new
bottlenecks) on my end when they were all independent. This meant it got
*developed* as a single monolithic set of changes, but merged as
logical commits in grouped and bisectable branches.

> Branch dependencies are also hard to handle 
>  from a mental/human perspective -- it's not just the tools -- better to 
> have independent branches. Or, to wait for the first branch to make it 
> all the way back to master before starting the second branch.

I think this is an artificial stall on my time to have to do this. If I
have a train of 3 branches to go in, that could take a week before I
could *start* the third versus get it all working at once and get a more
high-level view of earlier branches (not to mention it's probably real
testing of the earlier branches and bugs get found and fixed earlier
this way).

One place I see this a lot is ParaView changes which require changes in
VTK which are untestable without the ParaView end (see unified-bindings
or getting VTK's testing up to par so that ParaView can use the
infrastructure as well).

Context switching is already expensive; let's not force them because of
a tool deficiency if possible.

> I know, I know. git was supposed to eliminate the need for patience and 
> perseverence, right? ;-)

Not to my knowledge; just be more flexible to different work styles (of
which I'm aware mine is not the most common).

> >  - Comments are per patch set and not carried over if the commented-on
> >    code didn't change which makes fixing half the comments on a review
> >    a pain.
> 
> That is a pain -- but is it handled any better in one of the other 
> suggested workflows?

Github seems to attach comments to commit hashes and/or blob hashes, so
a rebase or amend will trash the linkings. Reviewboard *might* be able
to do it via its pure-diff orientation, but I'm not completely sure.

--Ben



More information about the vtk-developers mailing list