You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Arvind Prabhakar <ar...@apache.org> on 2012/02/19 00:44:15 UTC

[DISCUSS] Revising RTC policy

Hi,

Soon after Flume entered the Apache Incubator, we discussed and implemented
a Review-Then-Commit policy for contributing towards the project. Since
that time, this policy has served as well and continues to do so. The
formal specification of this policy can be found in the email below:

http://markmail.org/thread/wfjpauoffz67k6ut

Now that Flume has made its first release from the incubator, and the
number of contributors is starting to grow, I wish to propose a slight
revision to this policy. Specifically the revision being proposed will
amend the exiting policy as follows:

   - All patches must require at lease one +1 vote from a committer.
   - A patch authored by a committer should be committed to the source
   control by another committer who +1s the patch during review.
   - First provision for no review commit:
      - If a patch authored by a committer is not reviewed within three
      days of submission, the patch author must request prioritization of the
      review on the developer mailing list by other committers.
      - If another three days pass after a reminder and no one reviews the
      code, the committer may push the patch in.
      - If during any of this period a review is started by another
      committer, then no time-out applies and both the author must address any
      suggestions and concerns as necessary to get a +1 by the reviewing
      committer.
   - Second provision for new review commit:
      - When cutting a release, the Release Manager will have the authority
      to make commits to facilitate the release. Such commits should only be to
      address build and other infrastructure requirements as needed for the
      release.
      - Modifying a test or functionality necessary to cut a release would
      still require the regular review cycle and a minimum of one +1
from another
      committer.

Most of this provision is already part of the originally stated policy.
What this amendment does it to make explicit the requirement to have two
committers per patch that is authored by another committer. This will allow
us to balance our priorities and help keep more committers active on the
project.

If you have any concerns regarding this amendment, please bring them up for
discussion on this thread.

Thanks,
Arvind Prabhakar

Re: [DISCUSS] Revising RTC policy

Posted by Arvind Prabhakar <ar...@apache.org>.
Since the discussion has settled on this proposal, I will go ahead and call
a community vote on this.

Thanks,
Arvind Prabhakar

On Sat, Feb 18, 2012 at 3:44 PM, Arvind Prabhakar <ar...@apache.org> wrote:

> Hi,
>
> Soon after Flume entered the Apache Incubator, we discussed and
> implemented a Review-Then-Commit policy for contributing towards the
> project. Since that time, this policy has served as well and continues to
> do so. The formal specification of this policy can be found in the email
> below:
>
> http://markmail.org/thread/wfjpauoffz67k6ut
>
> Now that Flume has made its first release from the incubator, and the
> number of contributors is starting to grow, I wish to propose a slight
> revision to this policy. Specifically the revision being proposed will
> amend the exiting policy as follows:
>
>    - All patches must require at lease one +1 vote from a committer.
>    - A patch authored by a committer should be committed to the source
>    control by another committer who +1s the patch during review.
>    - First provision for no review commit:
>       - If a patch authored by a committer is not reviewed within three
>       days of submission, the patch author must request prioritization of the
>       review on the developer mailing list by other committers.
>       - If another three days pass after a reminder and no one reviews
>       the code, the committer may push the patch in.
>       - If during any of this period a review is started by another
>       committer, then no time-out applies and both the author must address any
>       suggestions and concerns as necessary to get a +1 by the reviewing
>       committer.
>    - Second provision for new review commit:
>       - When cutting a release, the Release Manager will have the
>       authority to make commits to facilitate the release. Such commits should
>       only be to address build and other infrastructure requirements as needed
>       for the release.
>       - Modifying a test or functionality necessary to cut a release
>       would still require the regular review cycle and a minimum of one +1 from
>       another committer.
>
> Most of this provision is already part of the originally stated policy.
> What this amendment does it to make explicit the requirement to have two
> committers per patch that is authored by another committer. This will allow
> us to balance our priorities and help keep more committers active on the
> project.
>
> If you have any concerns regarding this amendment, please bring them up
> for discussion on this thread.
>
> Thanks,
> Arvind Prabhakar
>
>
>

Re: [DISCUSS] Revising RTC policy

Posted by Brock Noland <br...@cloudera.com>.
+1 (non-binding)

This is a very high standard for commits, even higher than hadoop
(committers can commit their own changes). I think high standards are
good, my only concern is whether it will slow the pace of progress too
much. Should that occur we can always change the policy.

Cheers,
Brock

On Sun, Feb 19, 2012 at 1:41 AM, Jarek Jarcec Cecho <ja...@apache.org> wrote:
> Seems fine to me (contributor opinion).
>
> Jarcec
>
> On Sat, Feb 18, 2012 at 03:44:15PM -0800, Arvind Prabhakar wrote:
>> Hi,
>>
>> Soon after Flume entered the Apache Incubator, we discussed and implemented
>> a Review-Then-Commit policy for contributing towards the project. Since
>> that time, this policy has served as well and continues to do so. The
>> formal specification of this policy can be found in the email below:
>>
>> http://markmail.org/thread/wfjpauoffz67k6ut
>>
>> Now that Flume has made its first release from the incubator, and the
>> number of contributors is starting to grow, I wish to propose a slight
>> revision to this policy. Specifically the revision being proposed will
>> amend the exiting policy as follows:
>>
>>    - All patches must require at lease one +1 vote from a committer.
>>    - A patch authored by a committer should be committed to the source
>>    control by another committer who +1s the patch during review.
>>    - First provision for no review commit:
>>       - If a patch authored by a committer is not reviewed within three
>>       days of submission, the patch author must request prioritization of the
>>       review on the developer mailing list by other committers.
>>       - If another three days pass after a reminder and no one reviews the
>>       code, the committer may push the patch in.
>>       - If during any of this period a review is started by another
>>       committer, then no time-out applies and both the author must address any
>>       suggestions and concerns as necessary to get a +1 by the reviewing
>>       committer.
>>    - Second provision for new review commit:
>>       - When cutting a release, the Release Manager will have the authority
>>       to make commits to facilitate the release. Such commits should only be to
>>       address build and other infrastructure requirements as needed for the
>>       release.
>>       - Modifying a test or functionality necessary to cut a release would
>>       still require the regular review cycle and a minimum of one +1
>> from another
>>       committer.
>>
>> Most of this provision is already part of the originally stated policy.
>> What this amendment does it to make explicit the requirement to have two
>> committers per patch that is authored by another committer. This will allow
>> us to balance our priorities and help keep more committers active on the
>> project.
>>
>> If you have any concerns regarding this amendment, please bring them up for
>> discussion on this thread.
>>
>> Thanks,
>> Arvind Prabhakar



-- 
Apache MRUnit - Unit testing MapReduce - http://incubator.apache.org/mrunit/

Re: [DISCUSS] Revising RTC policy

Posted by Jarek Jarcec Cecho <ja...@apache.org>.
Seems fine to me (contributor opinion).

Jarcec

On Sat, Feb 18, 2012 at 03:44:15PM -0800, Arvind Prabhakar wrote:
> Hi,
> 
> Soon after Flume entered the Apache Incubator, we discussed and implemented
> a Review-Then-Commit policy for contributing towards the project. Since
> that time, this policy has served as well and continues to do so. The
> formal specification of this policy can be found in the email below:
> 
> http://markmail.org/thread/wfjpauoffz67k6ut
> 
> Now that Flume has made its first release from the incubator, and the
> number of contributors is starting to grow, I wish to propose a slight
> revision to this policy. Specifically the revision being proposed will
> amend the exiting policy as follows:
> 
>    - All patches must require at lease one +1 vote from a committer.
>    - A patch authored by a committer should be committed to the source
>    control by another committer who +1s the patch during review.
>    - First provision for no review commit:
>       - If a patch authored by a committer is not reviewed within three
>       days of submission, the patch author must request prioritization of the
>       review on the developer mailing list by other committers.
>       - If another three days pass after a reminder and no one reviews the
>       code, the committer may push the patch in.
>       - If during any of this period a review is started by another
>       committer, then no time-out applies and both the author must address any
>       suggestions and concerns as necessary to get a +1 by the reviewing
>       committer.
>    - Second provision for new review commit:
>       - When cutting a release, the Release Manager will have the authority
>       to make commits to facilitate the release. Such commits should only be to
>       address build and other infrastructure requirements as needed for the
>       release.
>       - Modifying a test or functionality necessary to cut a release would
>       still require the regular review cycle and a minimum of one +1
> from another
>       committer.
> 
> Most of this provision is already part of the originally stated policy.
> What this amendment does it to make explicit the requirement to have two
> committers per patch that is authored by another committer. This will allow
> us to balance our priorities and help keep more committers active on the
> project.
> 
> If you have any concerns regarding this amendment, please bring them up for
> discussion on this thread.
> 
> Thanks,
> Arvind Prabhakar

Re: [DISCUSS] Revising RTC policy

Posted by Arvind Prabhakar <ar...@apache.org>.
Hi Mike,

The proposal to amend RTC policy was based on my experience in other
projects so far, where I have observed such a policy to help build a
stronger community. That said, please see my comments to your
questions below:

> 1. Why would you want a second committer to have to commit
> your changes if they already gave a +1? And how do you think
> this could help keep more committers active on the project?

There is not much difference between doing a review and doing a review
and commit. The later does require you to have the latest codebase and
be able to run tests before you checkin the patch. Committers do it
all the time for contributions coming from community contributors.
Having a uniform standard of accepting patches from contributors and
committers alike promotes transparency in the project and helps the
community thrive.

> 2. Can you clarify how the below part of the proposed policy is different from what is in place today?
>> - If during any of this period a review is started by another
>> committer, then no time-out applies and both the author must address any
>> suggestions and concerns as necessary to get a +1 by the reviewing
>> committer.
>

The current policy requires at least one +1 in order to commit a
patch. It does not explicitly state that a +1 from one committer does
not bypass an on-going review from another committer. This part of the
proposed amendment was to make this requirement explicit. In practice,
I have not come across a situation where anyone has bypassed a
reviewer once they have the necessary +1 from some other reviewer.
Given that it is already a practice in our project, stating it
explicitly would only make our governance model explicit and
transparent.

Thanks,
Arvind Prabhakar


On Thu, Feb 23, 2012 at 8:52 PM, Mike Percy <mp...@cloudera.com> wrote:
> Hi Arvind,
>
> I was out of town when this discussion thread went out, but looking back at this discussion thread I didn't see called out a lot of specific reasons for proposing these changes. I was wondering if you wouldn't mind relating why you want to make some of these changes. For example:
>
> 1. Why would you want a second committer to have to commit your changes if they already gave a +1? And how do you think this could help keep more committers active on the project?
>
> 2. Can you clarify how the below part of the proposed policy is different from what is in place today?
>> - If during any of this period a review is started by another
>> committer, then no time-out applies and both the author must address any
>> suggestions and concerns as necessary to get a +1 by the reviewing
>> committer.
>
>
> Regards,
> Mike
>
> On Feb 18, 2012, at 3:44 PM, Arvind Prabhakar wrote:
>
>> Hi,
>>
>> Soon after Flume entered the Apache Incubator, we discussed and implemented
>> a Review-Then-Commit policy for contributing towards the project. Since
>> that time, this policy has served as well and continues to do so. The
>> formal specification of this policy can be found in the email below:
>>
>> http://markmail.org/thread/wfjpauoffz67k6ut
>>
>> Now that Flume has made its first release from the incubator, and the
>> number of contributors is starting to grow, I wish to propose a slight
>> revision to this policy. Specifically the revision being proposed will
>> amend the exiting policy as follows:
>>
>>   - All patches must require at lease one +1 vote from a committer.
>>   - A patch authored by a committer should be committed to the source
>>   control by another committer who +1s the patch during review.
>>   - First provision for no review commit:
>>      - If a patch authored by a committer is not reviewed within three
>>      days of submission, the patch author must request prioritization of the
>>      review on the developer mailing list by other committers.
>>      - If another three days pass after a reminder and no one reviews the
>>      code, the committer may push the patch in.
>>      - If during any of this period a review is started by another
>>      committer, then no time-out applies and both the author must address any
>>      suggestions and concerns as necessary to get a +1 by the reviewing
>>      committer.
>>   - Second provision for new review commit:
>>      - When cutting a release, the Release Manager will have the authority
>>      to make commits to facilitate the release. Such commits should only be to
>>      address build and other infrastructure requirements as needed for the
>>      release.
>>      - Modifying a test or functionality necessary to cut a release would
>>      still require the regular review cycle and a minimum of one +1
>> from another
>>      committer.
>>
>> Most of this provision is already part of the originally stated policy.
>> What this amendment does it to make explicit the requirement to have two
>> committers per patch that is authored by another committer. This will allow
>> us to balance our priorities and help keep more committers active on the
>> project.
>>
>> If you have any concerns regarding this amendment, please bring them up for
>> discussion on this thread.
>>
>> Thanks,
>> Arvind Prabhakar
>

Re: [DISCUSS] Revising RTC policy

Posted by Mike Percy <mp...@cloudera.com>.
Hi Arvind,

I was out of town when this discussion thread went out, but looking back at this discussion thread I didn't see called out a lot of specific reasons for proposing these changes. I was wondering if you wouldn't mind relating why you want to make some of these changes. For example:

1. Why would you want a second committer to have to commit your changes if they already gave a +1? And how do you think this could help keep more committers active on the project?

2. Can you clarify how the below part of the proposed policy is different from what is in place today?
> - If during any of this period a review is started by another
> committer, then no time-out applies and both the author must address any
> suggestions and concerns as necessary to get a +1 by the reviewing
> committer.


Regards,
Mike

On Feb 18, 2012, at 3:44 PM, Arvind Prabhakar wrote:

> Hi,
> 
> Soon after Flume entered the Apache Incubator, we discussed and implemented
> a Review-Then-Commit policy for contributing towards the project. Since
> that time, this policy has served as well and continues to do so. The
> formal specification of this policy can be found in the email below:
> 
> http://markmail.org/thread/wfjpauoffz67k6ut
> 
> Now that Flume has made its first release from the incubator, and the
> number of contributors is starting to grow, I wish to propose a slight
> revision to this policy. Specifically the revision being proposed will
> amend the exiting policy as follows:
> 
>   - All patches must require at lease one +1 vote from a committer.
>   - A patch authored by a committer should be committed to the source
>   control by another committer who +1s the patch during review.
>   - First provision for no review commit:
>      - If a patch authored by a committer is not reviewed within three
>      days of submission, the patch author must request prioritization of the
>      review on the developer mailing list by other committers.
>      - If another three days pass after a reminder and no one reviews the
>      code, the committer may push the patch in.
>      - If during any of this period a review is started by another
>      committer, then no time-out applies and both the author must address any
>      suggestions and concerns as necessary to get a +1 by the reviewing
>      committer.
>   - Second provision for new review commit:
>      - When cutting a release, the Release Manager will have the authority
>      to make commits to facilitate the release. Such commits should only be to
>      address build and other infrastructure requirements as needed for the
>      release.
>      - Modifying a test or functionality necessary to cut a release would
>      still require the regular review cycle and a minimum of one +1
> from another
>      committer.
> 
> Most of this provision is already part of the originally stated policy.
> What this amendment does it to make explicit the requirement to have two
> committers per patch that is authored by another committer. This will allow
> us to balance our priorities and help keep more committers active on the
> project.
> 
> If you have any concerns regarding this amendment, please bring them up for
> discussion on this thread.
> 
> Thanks,
> Arvind Prabhakar


Re: [DISCUSS] Revising RTC policy

Posted by Ralph Goers <ra...@dslextreme.com>.
The only comment I have is that this just feels very "heavy". If we were using git instead of svn and the revisions could more easily be integrated perhaps it wouldn't seem so.  However, this is the same comment I made regarding RTC in the first place so I'm not sure this changes anything in that regard.

Ralph

On Feb 18, 2012, at 3:44 PM, Arvind Prabhakar wrote:

> Hi,
> 
> Soon after Flume entered the Apache Incubator, we discussed and implemented
> a Review-Then-Commit policy for contributing towards the project. Since
> that time, this policy has served as well and continues to do so. The
> formal specification of this policy can be found in the email below:
> 
> http://markmail.org/thread/wfjpauoffz67k6ut
> 
> Now that Flume has made its first release from the incubator, and the
> number of contributors is starting to grow, I wish to propose a slight
> revision to this policy. Specifically the revision being proposed will
> amend the exiting policy as follows:
> 
>   - All patches must require at lease one +1 vote from a committer.
>   - A patch authored by a committer should be committed to the source
>   control by another committer who +1s the patch during review.
>   - First provision for no review commit:
>      - If a patch authored by a committer is not reviewed within three
>      days of submission, the patch author must request prioritization of the
>      review on the developer mailing list by other committers.
>      - If another three days pass after a reminder and no one reviews the
>      code, the committer may push the patch in.
>      - If during any of this period a review is started by another
>      committer, then no time-out applies and both the author must address any
>      suggestions and concerns as necessary to get a +1 by the reviewing
>      committer.
>   - Second provision for new review commit:
>      - When cutting a release, the Release Manager will have the authority
>      to make commits to facilitate the release. Such commits should only be to
>      address build and other infrastructure requirements as needed for the
>      release.
>      - Modifying a test or functionality necessary to cut a release would
>      still require the regular review cycle and a minimum of one +1
> from another
>      committer.
> 
> Most of this provision is already part of the originally stated policy.
> What this amendment does it to make explicit the requirement to have two
> committers per patch that is authored by another committer. This will allow
> us to balance our priorities and help keep more committers active on the
> project.
> 
> If you have any concerns regarding this amendment, please bring them up for
> discussion on this thread.
> 
> Thanks,
> Arvind Prabhakar