You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by Konstantin Shvachko <sh...@gmail.com> on 2015/02/27 22:04:59 UTC
DISCUSSION: Patch commit criteria.
There were discussions on several jiras and threads recently about how RTC
actually works in Hadoop.
My opinion has always been that for a patch to be committed it needs an
approval (+1) of at least one committer other than the author and no -1s.
The Bylaws seem to be stating just that:
"Consensus approval of active committers, but with a minimum of one +1."
See the full version under Actions / Code Change
<http://hadoop.apache.org/bylaws.html#Decision+Making>
Turned out people have different readings of that part of Bylaws, and
different opinions on how RTC should work in different cases. Some of the
questions that were raised include:
- Should we clarify the Code Change decision making clause in Bylaws?
- Should there be a relaxed criteria for "trivial" changes?
- Can a patch be committed if approved only by a non committer?
- Can a patch be committed based on self-review by a committer?
- What is the point for a non-committer to review the patch?
Creating this thread to discuss these (and other that I sure missed) issues
and to combine multiple discussions into one.
My personal opinion we should just stick to the tradition. Good or bad, it
worked for this community so far.
I think most of the discrepancies arise from the fact that reviewers are
hard to find. May be this should be the focus of improvements rather than
the RTC rules.
Thanks,
--Konst
Re: DISCUSSION: Patch commit criteria.
Posted by Konstantin Shvachko <sh...@gmail.com>.
Vinod,
I agree that triviality is hard to define and we should not add things that
can be interpreted multiple ways to the bylaws.
If something is not quite clear in the bylaws, it would make sense to have
a proposal of new phrasing, so that we could discuss it here and call a
vote upon reaching an agreement.
Thanks,
--Konst
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
Re: DISCUSSION: Patch commit criteria.
Posted by Karthik Kambatla <ka...@cloudera.com>.
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
In the past, I have made trivial (new lines, indentation, etc.) changes to
well reviewed patches before committing. Even then, I believe we should
upload the updated patch or the diff of trivial changes and wait for
someone else (potentially a non-committer contributor) to quickly check to
avoid making silly mistakes.
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
--
Karthik Kambatla
Software Engineer, Cloudera Inc.
--------------------------------------------
http://five.sentenc.es
Re: DISCUSSION: Patch commit criteria.
Posted by Karthik Kambatla <ka...@cloudera.com>.
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
In the past, I have made trivial (new lines, indentation, etc.) changes to
well reviewed patches before committing. Even then, I believe we should
upload the updated patch or the diff of trivial changes and wait for
someone else (potentially a non-committer contributor) to quickly check to
avoid making silly mistakes.
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
--
Karthik Kambatla
Software Engineer, Cloudera Inc.
--------------------------------------------
http://five.sentenc.es
Re: DISCUSSION: Patch commit criteria.
Posted by Karthik Kambatla <ka...@cloudera.com>.
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
In the past, I have made trivial (new lines, indentation, etc.) changes to
well reviewed patches before committing. Even then, I believe we should
upload the updated patch or the diff of trivial changes and wait for
someone else (potentially a non-committer contributor) to quickly check to
avoid making silly mistakes.
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
--
Karthik Kambatla
Software Engineer, Cloudera Inc.
--------------------------------------------
http://five.sentenc.es
Re: DISCUSSION: Patch commit criteria.
Posted by Konstantin Shvachko <sh...@gmail.com>.
Vinod,
I agree that triviality is hard to define and we should not add things that
can be interpreted multiple ways to the bylaws.
If something is not quite clear in the bylaws, it would make sense to have
a proposal of new phrasing, so that we could discuss it here and call a
vote upon reaching an agreement.
Thanks,
--Konst
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
Re: DISCUSSION: Patch commit criteria.
Posted by Karthik Kambatla <ka...@cloudera.com>.
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
In the past, I have made trivial (new lines, indentation, etc.) changes to
well reviewed patches before committing. Even then, I believe we should
upload the updated patch or the diff of trivial changes and wait for
someone else (potentially a non-committer contributor) to quickly check to
avoid making silly mistakes.
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
--
Karthik Kambatla
Software Engineer, Cloudera Inc.
--------------------------------------------
http://five.sentenc.es
Re: DISCUSSION: Patch commit criteria.
Posted by Konstantin Shvachko <sh...@gmail.com>.
Vinod,
I agree that triviality is hard to define and we should not add things that
can be interpreted multiple ways to the bylaws.
If something is not quite clear in the bylaws, it would make sense to have
a proposal of new phrasing, so that we could discuss it here and call a
vote upon reaching an agreement.
Thanks,
--Konst
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
Re: DISCUSSION: Patch commit criteria.
Posted by Konstantin Shvachko <sh...@gmail.com>.
Vinod,
I agree that triviality is hard to define and we should not add things that
can be interpreted multiple ways to the bylaws.
If something is not quite clear in the bylaws, it would make sense to have
a proposal of new phrasing, so that we could discuss it here and call a
vote upon reaching an agreement.
Thanks,
--Konst
On Mon, Mar 2, 2015 at 11:29 AM, Vinod Kumar Vavilapalli <
vinodkv@hortonworks.com> wrote:
> We always needed another committer's +1 even if it isn't that clear in the
> bylaws. In the minimum, we should codify this in the bylaws to avoid stuff
> like people committing their own patches.
>
> Regarding trivial changes, I always distinguish between trivial *patches*
> and trivial changes to *existing* patches. Patches even if trivial need to
> be +1ed by another committer. OTOH, many a times, for patches that are
> extensively reviewed, potentially for months on, I sometimes end up making
> a small javadoc/documentation change in the last version of patch before
> committing. It just avoids one more cycle and more delay. It's hard to
> codify this distinction though.
>
> Thanks
> +Vinod
>
> On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
> > There were discussions on several jiras and threads recently about how
> RTC
> > actually works in Hadoop.
> > My opinion has always been that for a patch to be committed it needs an
> > approval (+1) of at least one committer other than the author and no
> -1s.
> > The Bylaws seem to be stating just that:
> > "Consensus approval of active committers, but with a minimum of one +1."
> > See the full version under Actions / Code Change
> > <http://hadoop.apache.org/bylaws.html#Decision+Making>
> >
> > Turned out people have different readings of that part of Bylaws, and
> > different opinions on how RTC should work in different cases. Some of the
> > questions that were raised include:
> > - Should we clarify the Code Change decision making clause in Bylaws?
> > - Should there be a relaxed criteria for "trivial" changes?
> > - Can a patch be committed if approved only by a non committer?
> > - Can a patch be committed based on self-review by a committer?
> > - What is the point for a non-committer to review the patch?
> > Creating this thread to discuss these (and other that I sure missed)
> issues
> > and to combine multiple discussions into one.
> >
> > My personal opinion we should just stick to the tradition. Good or bad,
> it
> > worked for this community so far.
> > I think most of the discrepancies arise from the fact that reviewers are
> > hard to find. May be this should be the focus of improvements rather than
> > the RTC rules.
> >
> > Thanks,
> > --Konst
>
>
Re: DISCUSSION: Patch commit criteria.
Posted by Vinod Kumar Vavilapalli <vi...@hortonworks.com>.
We always needed another committer's +1 even if it isn't that clear in the bylaws. In the minimum, we should codify this in the bylaws to avoid stuff like people committing their own patches.
Regarding trivial changes, I always distinguish between trivial *patches* and trivial changes to *existing* patches. Patches even if trivial need to be +1ed by another committer. OTOH, many a times, for patches that are extensively reviewed, potentially for months on, I sometimes end up making a small javadoc/documentation change in the last version of patch before committing. It just avoids one more cycle and more delay. It's hard to codify this distinction though.
Thanks
+Vinod
On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com> wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
Re: DISCUSSION: Patch commit criteria.
Posted by "Colin P. McCabe" <cm...@apache.org>.
I agree with Andrew and Konst here. I don't think the language is
unclear in the rule, either... "consensus with a minimum of one +1"
clearly indicates that _other people_ are involved, not just one
person.
I would also mention that we created the "branch committer" role
specifically to make it easier to do rapid development on a new
feature. Branch committers can commit patches to a branch without any
full committers involved at all. When the branch is ready to merge,
the community can review it and give feedback.
best,
Colin
On Fri, Feb 27, 2015 at 2:27 PM, Andrew Wang <an...@cloudera.com> wrote:
> I have the same interpretation as Konst on this. +1 from at least one
> committer other than the author, no -1s.
>
> I don't think there should be an exclusion for trivial patches, since the
> definition of "trivial" is subjective. The exception here is CHANGES.txt,
> which is something we really should get rid of.
>
> Non-committers are still strongly encouraged to review patches even if
> their +1 is not binding. Additional reviews improve code quality. Also,
> when it comes to choosing new committers, one of the primary things I look
> for is a history of quality code reviews.
>
> Best,
> Andrew
>
> On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
>> There were discussions on several jiras and threads recently about how RTC
>> actually works in Hadoop.
>> My opinion has always been that for a patch to be committed it needs an
>> approval (+1) of at least one committer other than the author and no -1s.
>> The Bylaws seem to be stating just that:
>> "Consensus approval of active committers, but with a minimum of one +1."
>> See the full version under Actions / Code Change
>> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>>
>> Turned out people have different readings of that part of Bylaws, and
>> different opinions on how RTC should work in different cases. Some of the
>> questions that were raised include:
>> - Should we clarify the Code Change decision making clause in Bylaws?
>> - Should there be a relaxed criteria for "trivial" changes?
>> - Can a patch be committed if approved only by a non committer?
>> - Can a patch be committed based on self-review by a committer?
>> - What is the point for a non-committer to review the patch?
>> Creating this thread to discuss these (and other that I sure missed) issues
>> and to combine multiple discussions into one.
>>
>> My personal opinion we should just stick to the tradition. Good or bad, it
>> worked for this community so far.
>> I think most of the discrepancies arise from the fact that reviewers are
>> hard to find. May be this should be the focus of improvements rather than
>> the RTC rules.
>>
>> Thanks,
>> --Konst
>>
Re: DISCUSSION: Patch commit criteria.
Posted by "Colin P. McCabe" <cm...@apache.org>.
I agree with Andrew and Konst here. I don't think the language is
unclear in the rule, either... "consensus with a minimum of one +1"
clearly indicates that _other people_ are involved, not just one
person.
I would also mention that we created the "branch committer" role
specifically to make it easier to do rapid development on a new
feature. Branch committers can commit patches to a branch without any
full committers involved at all. When the branch is ready to merge,
the community can review it and give feedback.
best,
Colin
On Fri, Feb 27, 2015 at 2:27 PM, Andrew Wang <an...@cloudera.com> wrote:
> I have the same interpretation as Konst on this. +1 from at least one
> committer other than the author, no -1s.
>
> I don't think there should be an exclusion for trivial patches, since the
> definition of "trivial" is subjective. The exception here is CHANGES.txt,
> which is something we really should get rid of.
>
> Non-committers are still strongly encouraged to review patches even if
> their +1 is not binding. Additional reviews improve code quality. Also,
> when it comes to choosing new committers, one of the primary things I look
> for is a history of quality code reviews.
>
> Best,
> Andrew
>
> On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
>> There were discussions on several jiras and threads recently about how RTC
>> actually works in Hadoop.
>> My opinion has always been that for a patch to be committed it needs an
>> approval (+1) of at least one committer other than the author and no -1s.
>> The Bylaws seem to be stating just that:
>> "Consensus approval of active committers, but with a minimum of one +1."
>> See the full version under Actions / Code Change
>> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>>
>> Turned out people have different readings of that part of Bylaws, and
>> different opinions on how RTC should work in different cases. Some of the
>> questions that were raised include:
>> - Should we clarify the Code Change decision making clause in Bylaws?
>> - Should there be a relaxed criteria for "trivial" changes?
>> - Can a patch be committed if approved only by a non committer?
>> - Can a patch be committed based on self-review by a committer?
>> - What is the point for a non-committer to review the patch?
>> Creating this thread to discuss these (and other that I sure missed) issues
>> and to combine multiple discussions into one.
>>
>> My personal opinion we should just stick to the tradition. Good or bad, it
>> worked for this community so far.
>> I think most of the discrepancies arise from the fact that reviewers are
>> hard to find. May be this should be the focus of improvements rather than
>> the RTC rules.
>>
>> Thanks,
>> --Konst
>>
Re: DISCUSSION: Patch commit criteria.
Posted by "Colin P. McCabe" <cm...@apache.org>.
I agree with Andrew and Konst here. I don't think the language is
unclear in the rule, either... "consensus with a minimum of one +1"
clearly indicates that _other people_ are involved, not just one
person.
I would also mention that we created the "branch committer" role
specifically to make it easier to do rapid development on a new
feature. Branch committers can commit patches to a branch without any
full committers involved at all. When the branch is ready to merge,
the community can review it and give feedback.
best,
Colin
On Fri, Feb 27, 2015 at 2:27 PM, Andrew Wang <an...@cloudera.com> wrote:
> I have the same interpretation as Konst on this. +1 from at least one
> committer other than the author, no -1s.
>
> I don't think there should be an exclusion for trivial patches, since the
> definition of "trivial" is subjective. The exception here is CHANGES.txt,
> which is something we really should get rid of.
>
> Non-committers are still strongly encouraged to review patches even if
> their +1 is not binding. Additional reviews improve code quality. Also,
> when it comes to choosing new committers, one of the primary things I look
> for is a history of quality code reviews.
>
> Best,
> Andrew
>
> On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
>> There were discussions on several jiras and threads recently about how RTC
>> actually works in Hadoop.
>> My opinion has always been that for a patch to be committed it needs an
>> approval (+1) of at least one committer other than the author and no -1s.
>> The Bylaws seem to be stating just that:
>> "Consensus approval of active committers, but with a minimum of one +1."
>> See the full version under Actions / Code Change
>> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>>
>> Turned out people have different readings of that part of Bylaws, and
>> different opinions on how RTC should work in different cases. Some of the
>> questions that were raised include:
>> - Should we clarify the Code Change decision making clause in Bylaws?
>> - Should there be a relaxed criteria for "trivial" changes?
>> - Can a patch be committed if approved only by a non committer?
>> - Can a patch be committed based on self-review by a committer?
>> - What is the point for a non-committer to review the patch?
>> Creating this thread to discuss these (and other that I sure missed) issues
>> and to combine multiple discussions into one.
>>
>> My personal opinion we should just stick to the tradition. Good or bad, it
>> worked for this community so far.
>> I think most of the discrepancies arise from the fact that reviewers are
>> hard to find. May be this should be the focus of improvements rather than
>> the RTC rules.
>>
>> Thanks,
>> --Konst
>>
Re: DISCUSSION: Patch commit criteria.
Posted by "Colin P. McCabe" <cm...@apache.org>.
I agree with Andrew and Konst here. I don't think the language is
unclear in the rule, either... "consensus with a minimum of one +1"
clearly indicates that _other people_ are involved, not just one
person.
I would also mention that we created the "branch committer" role
specifically to make it easier to do rapid development on a new
feature. Branch committers can commit patches to a branch without any
full committers involved at all. When the branch is ready to merge,
the community can review it and give feedback.
best,
Colin
On Fri, Feb 27, 2015 at 2:27 PM, Andrew Wang <an...@cloudera.com> wrote:
> I have the same interpretation as Konst on this. +1 from at least one
> committer other than the author, no -1s.
>
> I don't think there should be an exclusion for trivial patches, since the
> definition of "trivial" is subjective. The exception here is CHANGES.txt,
> which is something we really should get rid of.
>
> Non-committers are still strongly encouraged to review patches even if
> their +1 is not binding. Additional reviews improve code quality. Also,
> when it comes to choosing new committers, one of the primary things I look
> for is a history of quality code reviews.
>
> Best,
> Andrew
>
> On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
> wrote:
>
>> There were discussions on several jiras and threads recently about how RTC
>> actually works in Hadoop.
>> My opinion has always been that for a patch to be committed it needs an
>> approval (+1) of at least one committer other than the author and no -1s.
>> The Bylaws seem to be stating just that:
>> "Consensus approval of active committers, but with a minimum of one +1."
>> See the full version under Actions / Code Change
>> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>>
>> Turned out people have different readings of that part of Bylaws, and
>> different opinions on how RTC should work in different cases. Some of the
>> questions that were raised include:
>> - Should we clarify the Code Change decision making clause in Bylaws?
>> - Should there be a relaxed criteria for "trivial" changes?
>> - Can a patch be committed if approved only by a non committer?
>> - Can a patch be committed based on self-review by a committer?
>> - What is the point for a non-committer to review the patch?
>> Creating this thread to discuss these (and other that I sure missed) issues
>> and to combine multiple discussions into one.
>>
>> My personal opinion we should just stick to the tradition. Good or bad, it
>> worked for this community so far.
>> I think most of the discrepancies arise from the fact that reviewers are
>> hard to find. May be this should be the focus of improvements rather than
>> the RTC rules.
>>
>> Thanks,
>> --Konst
>>
Re: DISCUSSION: Patch commit criteria.
Posted by Andrew Wang <an...@cloudera.com>.
I have the same interpretation as Konst on this. +1 from at least one
committer other than the author, no -1s.
I don't think there should be an exclusion for trivial patches, since the
definition of "trivial" is subjective. The exception here is CHANGES.txt,
which is something we really should get rid of.
Non-committers are still strongly encouraged to review patches even if
their +1 is not binding. Additional reviews improve code quality. Also,
when it comes to choosing new committers, one of the primary things I look
for is a history of quality code reviews.
Best,
Andrew
On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
>
Re: DISCUSSION: Patch commit criteria.
Posted by Andrew Wang <an...@cloudera.com>.
I have the same interpretation as Konst on this. +1 from at least one
committer other than the author, no -1s.
I don't think there should be an exclusion for trivial patches, since the
definition of "trivial" is subjective. The exception here is CHANGES.txt,
which is something we really should get rid of.
Non-committers are still strongly encouraged to review patches even if
their +1 is not binding. Additional reviews improve code quality. Also,
when it comes to choosing new committers, one of the primary things I look
for is a history of quality code reviews.
Best,
Andrew
On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
>
Re: DISCUSSION: Patch commit criteria.
Posted by Andrew Wang <an...@cloudera.com>.
I have the same interpretation as Konst on this. +1 from at least one
committer other than the author, no -1s.
I don't think there should be an exclusion for trivial patches, since the
definition of "trivial" is subjective. The exception here is CHANGES.txt,
which is something we really should get rid of.
Non-committers are still strongly encouraged to review patches even if
their +1 is not binding. Additional reviews improve code quality. Also,
when it comes to choosing new committers, one of the primary things I look
for is a history of quality code reviews.
Best,
Andrew
On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
>
Re: DISCUSSION: Patch commit criteria.
Posted by Vinod Kumar Vavilapalli <vi...@hortonworks.com>.
We always needed another committer's +1 even if it isn't that clear in the bylaws. In the minimum, we should codify this in the bylaws to avoid stuff like people committing their own patches.
Regarding trivial changes, I always distinguish between trivial *patches* and trivial changes to *existing* patches. Patches even if trivial need to be +1ed by another committer. OTOH, many a times, for patches that are extensively reviewed, potentially for months on, I sometimes end up making a small javadoc/documentation change in the last version of patch before committing. It just avoids one more cycle and more delay. It's hard to codify this distinction though.
Thanks
+Vinod
On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com> wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
Re: DISCUSSION: Patch commit criteria.
Posted by Andrew Wang <an...@cloudera.com>.
I have the same interpretation as Konst on this. +1 from at least one
committer other than the author, no -1s.
I don't think there should be an exclusion for trivial patches, since the
definition of "trivial" is subjective. The exception here is CHANGES.txt,
which is something we really should get rid of.
Non-committers are still strongly encouraged to review patches even if
their +1 is not binding. Additional reviews improve code quality. Also,
when it comes to choosing new committers, one of the primary things I look
for is a history of quality code reviews.
Best,
Andrew
On Fri, Feb 27, 2015 at 1:04 PM, Konstantin Shvachko <sh...@gmail.com>
wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
>
Re: DISCUSSION: Patch commit criteria.
Posted by Vinod Kumar Vavilapalli <vi...@hortonworks.com>.
We always needed another committer's +1 even if it isn't that clear in the bylaws. In the minimum, we should codify this in the bylaws to avoid stuff like people committing their own patches.
Regarding trivial changes, I always distinguish between trivial *patches* and trivial changes to *existing* patches. Patches even if trivial need to be +1ed by another committer. OTOH, many a times, for patches that are extensively reviewed, potentially for months on, I sometimes end up making a small javadoc/documentation change in the last version of patch before committing. It just avoids one more cycle and more delay. It's hard to codify this distinction though.
Thanks
+Vinod
On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com> wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst
Re: DISCUSSION: Patch commit criteria.
Posted by Vinod Kumar Vavilapalli <vi...@hortonworks.com>.
We always needed another committer's +1 even if it isn't that clear in the bylaws. In the minimum, we should codify this in the bylaws to avoid stuff like people committing their own patches.
Regarding trivial changes, I always distinguish between trivial *patches* and trivial changes to *existing* patches. Patches even if trivial need to be +1ed by another committer. OTOH, many a times, for patches that are extensively reviewed, potentially for months on, I sometimes end up making a small javadoc/documentation change in the last version of patch before committing. It just avoids one more cycle and more delay. It's hard to codify this distinction though.
Thanks
+Vinod
On Feb 27, 2015, at 1:04 PM, Konstantin Shvachko <sh...@gmail.com> wrote:
> There were discussions on several jiras and threads recently about how RTC
> actually works in Hadoop.
> My opinion has always been that for a patch to be committed it needs an
> approval (+1) of at least one committer other than the author and no -1s.
> The Bylaws seem to be stating just that:
> "Consensus approval of active committers, but with a minimum of one +1."
> See the full version under Actions / Code Change
> <http://hadoop.apache.org/bylaws.html#Decision+Making>
>
> Turned out people have different readings of that part of Bylaws, and
> different opinions on how RTC should work in different cases. Some of the
> questions that were raised include:
> - Should we clarify the Code Change decision making clause in Bylaws?
> - Should there be a relaxed criteria for "trivial" changes?
> - Can a patch be committed if approved only by a non committer?
> - Can a patch be committed based on self-review by a committer?
> - What is the point for a non-committer to review the patch?
> Creating this thread to discuss these (and other that I sure missed) issues
> and to combine multiple discussions into one.
>
> My personal opinion we should just stick to the tradition. Good or bad, it
> worked for this community so far.
> I think most of the discrepancies arise from the fact that reviewers are
> hard to find. May be this should be the focus of improvements rather than
> the RTC rules.
>
> Thanks,
> --Konst