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

Bill Lorensen bill.lorensen at gmail.com
Mon Aug 25 16:28:45 EDT 2014 

Robert,

A +1. To me it looks like our current gerrit workflow.

On Mon, Aug 25, 2014 at 3:27 PM, Robert Maynard
<robert.maynard at kitware.com> wrote:
> My preferred workflow as a developer would be:
>
> 1. Create a topic / pull request.
> 2. I assign people to review topic
> 3. Automatic integration builds start going to work.
> 4. Something reports back to the status of the build and tests, which
> notifies the author
>     and reviewers that building and testing have finished.
> 5. Reviewers decide based on code and build status if code is worth merging.
>
> When changes happen:
> 1. I extend the topic with fixes based on review or integration tests,
> or squash changes back
>     into the original topic as they are style/spelling issues.
> 2. Automatic integration starts up again
> 3. The commenters are notified that author has modified the topic
> 4. Commenters can easily see the difference between the old and new versions.
>
>
> As a reviewer I desire a different set of features:
>
> 1. Ability to comment inline with the code
> 2. Easy way to generate a single view of all the changes for a topic
> 3. Ability to easily see if the topic has broken the build or tests.
> 5. A way to force a re-build or re-test of a topic incase I believe something
>    was caused by a machine failure
> 6. Ability to bring other people into the conversation. Be it through
> '@' tags or
>    adding people as reviewers.
> 7. Ability to get emails on a per project / topic level
>
>
> On Mon, Aug 25, 2014 at 2:34 PM, Berk Geveci <berk.geveci at kitware.com> wrote:
>> I encourage everyone to think in terms of the workflow(s) that we prefer
>> rather than tools. If we start with "can we live with vanilla gerrit", the
>> answer is going to be yes for most people. However, this is only part of the
>> picture. Some folks mentioned bug tracker integration, others support for
>> rich content such as markdown etc. Integration with cdash @ home is an
>> obvious one. These are good examples of what we want in workflows. Once we
>> have a decent understanding of what people want the workflow to be, we can
>> discuss tools more deeply.
>>
>> So I am interpreting Utkarsh's email as "Thus for my personal sake,
>> squashing to a single commit is a non-starter". And yes, we can manage
>> multiple changeset topics in Gerrit. But we can also satisfy Utkarsh's
>> workflow requirements using pretty much any other tool be it
>> Github/Gitlab/Bitbucket or even mailing list.
>>
>> Since we have to migrate, let's use it as an opportunity and think about
>> upgrading our overall workflow.
>>
>> -berk
>>
>>
>> On Mon, Aug 25, 2014 at 1:24 PM, Utkarsh Ayachit
>> <utkarsh.ayachit at kitware.com> wrote:
>>>
>>> As I was reading this I tried to mull on why I don't like our current
>>> gerrit to see if that'd help me figure out what my vote would be.
>>>
>>> We want to adopt the same workflow for ParaView too, and I'm afraid
>>> squashing all commits into a single commit just doesn't work the several of
>>> changes which are core and critical to the evolution of the software. Thus
>>> for my personal sake, squashing to a single commit is a non-starter.
>>>
>>> For the most part, VTK-gerrit does everything I'd want it to do. All my
>>> complaints are really user-interface related -- I'm sure everyone has run
>>> into them so there's no point hashing those out here. I started looking
>>> around for what other gerrit review users are doing and I was pleasantly
>>> surprised, on the face. I see that most other gerrit pages look much nicer
>>> and cleaner than ours.
>>> e.g.
>>>
>>> https://codereview.qt-project.org/#/q/status:open,n,z
>>> https://review.openstack.org/#/q/status:open,n,z
>>> http://review.couchbase.org/#/q/status:open,n,z
>>> https://gwt-review.googlesource.com/#/q/status:open
>>>
>>> As I understand, we're using a very old version of gerrit since we had to
>>> add support for topic branches. Several of the UI issues do indeed stem from
>>> interacting with topic branches. Looking for topic branches in gerrit, I
>>> found this:
>>> https://wiki.openstack.org/wiki/Gerrit_Workflow#Long-lived_Topic_Branches
>>>
>>> Looking at openstack gerrit, we do see support for topic branches too!
>>>
>>>
>>> https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/l3-high-availability,n,z
>>>
>>> The notable difference seems that you can't "review" a topic, you "review"
>>> commits in a topic", but in essence that's what we do on VTK-gerrit too,
>>> anyways.
>>>
>>> Utkarsh
>>>
>>>
>>>
>>>
>>> On Mon, Aug 25, 2014 at 10:53 AM, Berk Geveci <berk.geveci at kitware.com>
>>> wrote:
>>>>
>>>> Bill, David: Can you provide some details on which parts of the Vanilla
>>>> Gerrit workflow you like? I'd like to understand if these are unique to the
>>>> Gerrit tool. As far as I can tell, things that are somewhat unique to this
>>>> workflow are:
>>>>
>>>> 1. Encouragement of very short branches, single changesets preferred,
>>>> 2. Voting (although bitbucket has something like this too),
>>>> 3. Assignment of multiple reviewers,
>>>> 4. Keeping around all revisions of a changeset that were created during
>>>> the review for future audit (Github does not have this, not clear if others
>>>> have it)
>>>>
>>>> Did I miss anything? Which of these are important to prefer Gerrit over
>>>> other tools?
>>>>
>>>> Best,
>>>> -berk
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> -berk
>>>>
>>>>
>>>> On Sun, Aug 24, 2014 at 12:52 PM, Bill Lorensen <bill.lorensen at gmail.com>
>>>> wrote:
>>>>>
>>>>> I prefer Vanilla Gerrit. I always liked single commits. And, ITK is
>>>>> using pretty much the vanilla workflow.
>>>>>
>>>>>
>>>>> On Sun, Aug 24, 2014 at 8:37 AM, Berk Geveci <berk.geveci at kitware.com>
>>>>> wrote:
>>>>> > Hi David,
>>>>> >
>>>>> > From what I can tell, you can't even get the order of changesets in a
>>>>> > topic
>>>>> > in vanilla Gerrit. You can search by a topic but that seems to be it
>>>>> > for
>>>>> > topic support. Also, I don't think that there is any way of merging an
>>>>> > entire topic. That's changeset based too.
>>>>> >
>>>>> > So based on what I see, I disagree with " topics in vanilla gerrit
>>>>> > really
>>>>> > aren't that bad" :-) If we were to use Gerrit, I think that we would
>>>>> > have to
>>>>> > require that branches are squashed to 1 commit except unusual
>>>>> > circumstances.
>>>>> >
>>>>> > So a clarification to what I meant by vanilla Gerrit workflow: for the
>>>>> > most
>>>>> > part, the workflow would require branches of single or at most a few
>>>>> > commits. Longer running branches with 10s of commits would be a major
>>>>> > pain
>>>>> > to manage in Gerrit and would probably require special handling.
>>>>> >
>>>>> > -berk
>>>>> >
>>>>> >
>>>>> > On Sat, Aug 23, 2014 at 10:18 PM, David Gobbi <david.gobbi at gmail.com>
>>>>> > wrote:
>>>>> >>
>>>>> >> I'm all for sticking with gerrit.  I'll be sad to lose the topic
>>>>> >> review interface, but topics in vanilla gerrit really aren't that
>>>>> >> bad.
>>>>> >> We'll still be able to push topics and we'll be able to see all the
>>>>> >> changes that make up a topic.  I believe that what we lose is 1) the
>>>>> >> ability to do a topic-level review and 2) the ability to browse by
>>>>> >> topic.
>>>>> >>
>>>>> >> The only thing that annoys me with gerrit is that when you click
>>>>> >> "review", it hides all of the previous review comments until you're
>>>>> >> done. Hopefully the new gerrit has fixed this.
>>>>> >>
>>>>> >> The things I'd like to see in a code review system are:
>>>>> >>
>>>>> >> - Tight integration with the bugtracker.
>>>>> >>
>>>>> >> - Some kind of incentive for reviewers, even if it's just gold stars
>>>>> >> or a "top reviewer" list.
>>>>> >>
>>>>> >> Or we could be draconian and enforce a review/submit ratio, just like
>>>>> >> the old BBS's enforced upload/download ratios.
>>>>> >>
>>>>> >>  - David
>>>>> >
>>>>> >
>>>>> >
>>>>> > _______________________________________________
>>>>> > 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
>>>>> >
>>>>> >
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>>
>>>>
>>>
>>
>>
>> _______________________________________________
>> 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
>>
>>
> _______________________________________________
> 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
>



-- 
Unpaid intern in BillsBasement at noware dot com

More information about the vtk-developers mailing list