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

Wed Aug 27 09:32:40 EDT 2014

Here is my two cents/pence, I apologize if I am rehashing already discussed
topics, I am a little late to the party. In no particular order …

   -

   The development process should be enforced and supported by the review
   tool rather than people and documents. No one wants to be the  ‘enforcer’
   and documents often go unread.
   -

   GitHub/GitLab PR workflow works well with projects that have a
   ‘benevolent dictator’ and relatively small teams, I’m not sure it scales so
   well when you have a larger number of people with merge access where more
   coordination is necessary.
   -

   As well as indicating approval, it is just as important to be able
   indicate that you don’t want something to be merged, for example a work in
   progress branch or you don’t agree with how something should be
   implemented. The likes of GitHub/GitLab tend to have an all or nothing
   approach here, if I have merge privileges I can merge any PR in the project
   and no one can stop me short of removing my privileges.
   -

   In my opinion no one should have push access to master, all merging
   should be done automatically. My understanding with GitHub/GitLab is if you
   can merge in the UI you can push to master. I have been in projects where
   developers have push to master by mistake, it happens.
   -

   Long running topic with lots of commits should be the exception rather
   than the rule. Topics that have a lot of commits are hard to review, in my
   opinion these could be treated differently as the usefulness of the review
   tool breaks down here. For VTK the mean number of changes in a topic: 1.88
   and the median number of changes in a topic: 1. This to me indicates that
   topic branch support is a nice to have rather than a necessity.
   -

   I feel that as a part of the open source community we should be using
   open source tools where possible. So I would add being open source as a
   nice to have.
   - Finally I think supporting PR from GitHub is important, it's a
   familiar workflow for many developers. However, this doesn't mean that
   GitHub has to be our review tool. Integration is possible with other tools,
   for example Gerrit has a GitHub plugin. I have contributed to projects that
   use SVN through GitHub PRs :-)

Regards,

Chris

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

> Berk,
>
> I think we need to reach out to potential developers. Especially those
> outside of Kitware(and their paying customers) and the long term VTK
> developers outside Kitware. Those communities can adapt to anything.
> We need to focus on is how can we can attract new developers. In the
> past, new processes were adopted and adapted by Kitware, their
> customers and hard core VTK developers with very little input from the
> broader community of potential developers.
>
> ITK is going through the same issues but addressing the issues not
> through process change. They are looking at outreach and better
> documentation of the current process. Matt McCormick at Kitware has
> been leading this effort.
>
> I think there are lots of non-process improvements possible. But I
> don't have a silver bullet for attracting new developers. Perhaps VTK
> is too old school for today's developers. Stuck with an old
> architecture, old graphics architecture, old and complex languages. I
> honestly don't know what the root causes are. If we only include the
> old-timers in theses discussion then we will not attract a younger set
> of devleopers.
>
> Bill
>
> On Tue, Aug 26, 2014 at 4:09 PM, Bill Lorensen <bill.lorensen at gmail.com>
> wrote:
> > I was addressing the first requirements list. Ease of use is subjective.
> >
> > On Tue, Aug 26, 2014 at 4:04 PM, Berk Geveci <berk.geveci at kitware.com>
> wrote:
> >> 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
> >>
> >>
> >
> >
> >
> > --
> > Unpaid intern in BillsBasement at noware dot com
>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
> _______________________________________________
> 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://public.kitware.com/mailman/listinfo/vtk-developers
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20140827/894c4e68/attachment-0002.html>