Understanding the commit-then-review workflow

[Eric Schultz <es...@prplfoundation.org>]

All,

I'm trying to understand to the Apache Foundation model of voting in the
commit-then-review system. If a project is running on a CTR system and
someone says they dislike a piece of a previous commit, what happens? Does
it require consensus to remove the code or is the code removed if consensus
isn't reached to keep it in?

Thanks,

Eric

-- 
Eric Schultz, Community Manager, prpl Foundation
http://www.prplfoundation.org
eschultz@prplfoundation.org
cell: 920-539-0404
skype: ericschultzwi
@EricPrpl

Re: Understanding the commit-then-review workflow

[Rob Vesse <rv...@dotnetrdf.org>]

A practical example of a technical veto I've seen was when we added some
new optimisations that interacted badly with a pre-existing API in some
rare corner cases.

This actually went into a release and it wasn't until a particularly vocal
community member got round to upgrading that we became aware of the issue.
 As it was central to a their product stack and that product stack is
widely used in our field both by ourselves and the wider community this
was an appropriate veto.  After some heated discussion (because at first
we didn't understand why this was a problem - this was an example of when
a minimal test case is useless without the wider context) we were able to
come up with a technical solution that preserved the new optimisations and
resolved the bad interactions.

Rob

On 09/07/2014 08:12, "Mark Struberg" <st...@yahoo.de> wrote:

>I think the vetoing -1 from PMCs is mainly used for 'legal' reasons. If
>e.g. some new committer adds code which he took from an external project
>and it's license is not appropriate.
>I've not yet seen -1 for purely technical reasons. This might happen. But
>usually a consensus is reached after the pros and cons got discussed on
>the list.
>
>LieGrue,
>strub
>
>
>On Wednesday, 9 July 2014, 3:36, Justin Mclean <ju...@classsoftware.com>
>wrote:
> 
>
>>
>>
>>Hi,
>>
>>> Ugh.  That looks garbled to me.  What exactly is a "code modification
>>>vote?"  Any committer should be allowed to -1 a commit (with reasons)
>>
>>Any committer can vote -1 it's just not normally binding (depending on
>>project guidelines), I certainly can't see it being ignored when it does
>>happen, even a -1 by a user is probably trying to tell you something is
>>up :-) There was a long discussion about this when we were drafting up
>>the Apache Flex guidelines as the default rules are not always clear.
>>The was an attempt to get this wording fixed up but not much come of it.
>>
>>Thanks,
>>
>>Justin
>>

Re: Understanding the commit-then-review workflow

[Mark Struberg <st...@yahoo.de>]

I think the vetoing -1 from PMCs is mainly used for 'legal' reasons. If e.g. some new committer adds code which he took from an external project and it's license is not appropriate.
I've not yet seen -1 for purely technical reasons. This might happen. But usually a consensus is reached after the pros and cons got discussed on the list.

LieGrue,
strub


On Wednesday, 9 July 2014, 3:36, Justin Mclean <ju...@classsoftware.com> wrote:
 

>
>
>Hi,
>
>> Ugh.  That looks garbled to me.  What exactly is a "code modification vote?"  Any committer should be allowed to -1 a commit (with reasons)
>
>Any committer can vote -1 it's just not normally binding (depending on project guidelines), I certainly can't see it being ignored when it does happen, even a -1 by a user is probably trying to tell you something is up :-) There was a long discussion about this when we were drafting up the Apache Flex guidelines as the default rules are not always clear. The was an attempt to get this wording fixed up but not much come of it.
>
>Thanks,
>
>Justin
>
>

Re: Understanding the commit-then-review workflow

[Justin Mclean <ju...@classsoftware.com>]

Hi,

> Ugh.  That looks garbled to me.  What exactly is a "code modification vote?"  Any committer should be allowed to -1 a commit (with reasons)

Any committer can vote -1 it's just not normally binding (depending on project guidelines), I certainly can't see it being ignored when it does happen, even a -1 by a user is probably trying to tell you something is up :-) There was a long discussion about this when we were drafting up the Apache Flex guidelines as the default rules are not always clear. The was an attempt to get this wording fixed up but not much come of it.

Thanks,
Justin

Re: Understanding the commit-then-review workflow

[Bertrand Delacretaz <bd...@apache.org>]

On Wed, Jul 9, 2014 at 3:03 AM, Phil Steitz <ph...@gmail.com> wrote:
> ....Any committer should be allowed to -1 a commit (with reasons) and whoever made the commit should have to either
> revert or answer the reasons and gain consensus...

That's my understanding of the "Vetos" section at
http://www.apache.org/foundation/voting.html

That page says that a "qualified voter" can -1 a  commit, technically
I suppose this means a PMC member, but I'd say -1s coming from
contributors should be processed in the same way.

Anyway, those -1s should be very rare, if people see them often that
means IMO something's wrong with the project - or with its code
structure, as modularization often helps by reducing the surface on
which people need to agree. Moving to RTC for the critical parts of a
project, while staying on CTR on other parts, can also help.

-Bertrand

Re: Understanding the commit-then-review workflow

[Phil Steitz <ph...@gmail.com>]

> On Jul 8, 2014, at 3:29 PM, Justin Mclean <ju...@classsoftware.com> wrote:
> 
> Hi,
> 
>> Typically in the Apache projects, committers (folks elected by the project who have commit
>> privileges) have binding vetoes.
> 
> Actually the default is that only PMC members can veto code changes see under "Binding votes" from [1], but project guidelines may state otherwise.
> 
> Justin
> 
> 1. http://www.apache.org/foundation/voting.html

Ugh.  That looks garbled to me.  What exactly is a "code modification vote?"  Any committer should be allowed to -1 a commit (with reasons) and whoever made the commit should have to either revert or answer the reasons and gain consensus. 

Re: Understanding the commit-then-review workflow

[Justin Mclean <ju...@classsoftware.com>]

Hi,

> Typically in the Apache projects, committers (folks elected by the project who have commit
> privileges) have binding vetoes. 

Actually the default is that only PMC members can veto code changes see under "Binding votes" from [1], but project guidelines may state otherwise.

Justin

1. http://www.apache.org/foundation/voting.html

Re: Understanding the commit-then-review workflow

[David Nalley <da...@gnsa.us>]

On Tue, Jul 8, 2014 at 3:24 PM, Eric Schultz
<es...@prplfoundation.org> wrote:
> All,
>
> I'm trying to understand to the Apache Foundation model of voting in the
> commit-then-review system. If a project is running on a CTR system and
> someone says they dislike a piece of a previous commit, what happens? Does
> it require consensus to remove the code or is the code removed if consensus
> isn't reached to keep it in?
>
> Thanks,
>
> Eric
>

Hi Eric:

So version control helps here, and we tend to refer to outright
objection as a veto. So first, it depends on who is 'disliking' the
particular change or casting the veto. Typically in the Apache
projects, committers (folks elected by the project who have commit
privileges) have binding vetoes. We also only allow vetoes on code for
technical reasons. Generally the original committer will revert the
code if there is a veto. Occasionally the veto is withdrawn after
discussion, but vetoes can't be overridden.

HTH,

--David

Re: Understanding the commit-then-review workflow

[Phil Steitz <ph...@gmail.com>]

> On Jul 8, 2014, at 3:17 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
> 
> The idea of CTR is that the repo that the commit is made
> to is not in the direct path to a release. Thus, one
> can commit to the repo/branch as a sort of shared sandbox.

Not necessarily.  Some projects do CTR right up to release tags.

Phil


> 
> When a commit is proposed to enter into the release
> path, that path is RTC, which means that the patch
> must be proposed as a backport, and that before that
> backport commit can happen (eg, via svn merge), that
> the patch must be reviewed and voted on in that
> path (and on the mailing list specific to that path,
> if one exist).
> 
> Key in the below is how "dislike" is defined. If, for example,
> I don't like using a while() loop instead of a for() loop,
> I can either patch the code myself (since we are CTR) or
> offer suggestions on why the change should be made, etc...
> I cannot, however, veto the entire patch/commit for personal,
> non-technical reasons.
> 
> Now all this works only when developers are actively
> working *together* as a group, and that consensus building
> is a main factor in that development. That is why the
> *behavior* around git (not git itself) is somewhat circumspect
> @ Apache, since it really reinforces the idea that instead
> of it being a group of people working on the same codebase,
> each person is individually working on their own fork of
> the codebase and, eventually, some stuff gets shared. In
> that mindset, each patch/commit is seen as "personal" and
> not "communal", if you get my drift.
> 
>> On Jul 8, 2014, at 3:24 PM, Eric Schultz <es...@prplfoundation.org> wrote:
>> 
>> All,
>> 
>> I'm trying to understand to the Apache Foundation model of voting in the
>> commit-then-review system. If a project is running on a CTR system and
>> someone says they dislike a piece of a previous commit, what happens? Does
>> it require consensus to remove the code or is the code removed if consensus
>> isn't reached to keep it in?
>> 
>> Thanks,
>> 
>> Eric
>> 
>> -- 
>> Eric Schultz, Community Manager, prpl Foundation
>> http://www.prplfoundation.org
>> eschultz@prplfoundation.org
>> cell: 920-539-0404
>> skype: ericschultzwi
>> @EricPrpl
> 

Re: Understanding the commit-then-review workflow

[Jim Jagielski <ji...@jaguNET.com>]

The idea of CTR is that the repo that the commit is made
to is not in the direct path to a release. Thus, one
can commit to the repo/branch as a sort of shared sandbox.

When a commit is proposed to enter into the release
path, that path is RTC, which means that the patch
must be proposed as a backport, and that before that
backport commit can happen (eg, via svn merge), that
the patch must be reviewed and voted on in that
path (and on the mailing list specific to that path,
if one exist).

Key in the below is how "dislike" is defined. If, for example,
I don't like using a while() loop instead of a for() loop,
I can either patch the code myself (since we are CTR) or
offer suggestions on why the change should be made, etc...
I cannot, however, veto the entire patch/commit for personal,
non-technical reasons.

Now all this works only when developers are actively
working *together* as a group, and that consensus building
is a main factor in that development. That is why the
*behavior* around git (not git itself) is somewhat circumspect
@ Apache, since it really reinforces the idea that instead
of it being a group of people working on the same codebase,
each person is individually working on their own fork of
the codebase and, eventually, some stuff gets shared. In
that mindset, each patch/commit is seen as "personal" and
not "communal", if you get my drift.

On Jul 8, 2014, at 3:24 PM, Eric Schultz <es...@prplfoundation.org> wrote:

> All,
> 
> I'm trying to understand to the Apache Foundation model of voting in the
> commit-then-review system. If a project is running on a CTR system and
> someone says they dislike a piece of a previous commit, what happens? Does
> it require consensus to remove the code or is the code removed if consensus
> isn't reached to keep it in?
> 
> Thanks,
> 
> Eric
> 
> -- 
> Eric Schultz, Community Manager, prpl Foundation
> http://www.prplfoundation.org
> eschultz@prplfoundation.org
> cell: 920-539-0404
> skype: ericschultzwi
> @EricPrpl