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

Berk Geveci berk.geveci at kitware.com
Tue Aug 26 16:04:33 EDT 2014


I disagree. Gerrit does not support any of the "nice to have"s in a
straightforward way. Neither does vanilla Gerrit properly support "branch /
topic based workflow" in a straightforward way. It supports a changeset
based workflow and "branch / topic" based workflows have to be shoehorned
into it if a "branch / topic" has more than 1 changeset. Also to support
automated testing of branch tips, we would have to create custom
scaffolding since vanilla Gerrit has no such concept.

Gerrit, with a little help from cdash @ home does a decent job of the
following:

- Automated testing before merge (required to pass)
- Assign reviewers to topic
- Review / approval before merge (required to pass)
- Ability to go back to discussion leading to merge (audit trail)
- Automatic notification on change
- Ability to comment on the code (Web GUI preferred)

It clearly doesn't do any of this:

- Tight integration with issue (bug) tracking and release process
- Integration with Wiki
- Easy documentation / Markdown /rST support
- Easy way to generate single view of all changes in the Web GUI

In my opinion, it does a very poor job of these:

- Branch / topic based workflow
- Ease of use

The other requirements and nice-to-haves are independent of tools and can
be achieved using whatever tool. However, Mantis is also not the easiest
tool so replacing it is also a good idea.

In my opinion, the biggest weaknesses of Github and Gitlab are:

- Assign reviewers to topic
- Review / approval before merge (required to pass)
- Ability to go back to discussion leading to merge (audit trail)

They don't have a clear voting system and are based on more a general
discussion workflow. We would have to create some guidelines on how to
achieve these in those tools. For example, someone doing a pull request
would have to add a comment mentioning potential reviewers with the @name
syntax to "assign reviewers". The reviewers would have to use some
previously agreed upon language to approve a topic in the discussion.
Something like "Approved for merge".

Github is far superior to Gerrit in these:

- Branch / topic based workflow
- Automated testing before merge (required to pass) - tight integration
with Travis and demonstrated integration with cdash @ home through custom
hooks
- Automatic notification on change - much finer notification control
- Ability to comment on the code (Web GUI preferred) - go check it out if
you don't believe me
- Tight integration with issue (bug) tracking and release process
- Ease of use
- Integration with Wiki
- Easy documentation / Markdown /rST support
- Easy way to generate single view of all changes in the Web GUI - this is
impossible in Gerrit even for a single changeset

Best,
-berk





On Tue, Aug 26, 2014 at 3:43 PM, Bill Lorensen <bill.lorensen at gmail.com>
wrote:

> +1 and Gerrit seems to support them all.
>
>
> On Tue, Aug 26, 2014 at 3:32 PM, Berk Geveci <berk.geveci at kitware.com>
> wrote:
> > Here is a summary that I came up with from the discussion so far. Does
> this
> > look good?
> >
> > Requirements:
> >
> > - Branch / topic based workflow
> > - Automated testing before merge (required to pass)
> > - Assign reviewers to topic
> > - Review / approval before merge (required to pass)
> > - Ability to go back to discussion leading to merge (audit trail)
> > - Automatic notification on change
> > - Ability to comment on the code (Web GUI preferred)
> > - All reported bugs should be assessed and assigned
> >
> > Nice to have:
> >
> > - Tight integration with issue (bug) tracking and release process
> > - Stakeholders for particular pieces identified / in the loop / easy or
> > automatic assignment of
> > reviewers
> > - Ease of use
> > - Incentive for reviewers (goal being encouraging more reviews)
> > - Integration with Wiki
> > - Easy documentation / Markdown /rST support
> > - Easy way to generate single view of all changes in the Web GUI
> > - Lightweight proposal process for large changes
> > - Way to track performance regression
> >
>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20140826/387b310c/attachment-0002.html>


More information about the vtk-developers mailing list