You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Gleb Kanterov <gl...@spotify.com> on 2019/03/12 17:28:37 UTC
Always get to LGTM in Committer Guide
Before pressing merge button I was familiarizing myself with committer
guide [1]. It's saying:
> A committer (who is not the author of the code) should signal this either
by GitHub “approval” or by a comment such as “Looks good to me!” (LGTM).
Any committer can then merge the pull request. It is fine for a committer
to self-merge if another committer has reviewed the code and approved it,
just be sure to be explicit about whose job it is!
As I understand it, it's saying that the reviewer should be Beam Committer.
However, I remember from my personal experience and reading "An approach to
community building from Apache Beam" [2] that
> either the reviewer or the author be a committer
I'm wondering if we could rephrase our Commiter Guide a bit to make it
clear.
[1]: https://beam.apache.org/contribute/committer-guide/
[2]: https://blogs.apache.org/comdev/entry/an-approach-to-community-building
Re: Always get to LGTM in Committer Guide
Posted by Kenneth Knowles <ke...@apache.org>.
Indeed. I've opened a PR to record the change:
https://github.com/apache/beam/pull/8040
Kenn
On Tue, Mar 12, 2019 at 10:43 AM Rui Wang <ru...@google.com> wrote:
> Yes. I think we changed the policy in [1].
>
> Quote from the thread:
>
> (1) At least one committer is involved with the code review, as either a
> reviewer or as the author
> (2) A contributor has approved the change
>
> prior to merging any change.
>
> This changes our policy from its current requirement that at least one
> committer *who is not the author* has approved the change prior to merging.
> We believe that changing this process will improve code review throughput,
> reduce committer load, and engage more of the community in the code review
> process.
>
>
>
> [1]:
> https://lists.apache.org/thread.html/34a10b9d4d0b3b1cc92132779ab505bcb4a759aa9ae40f3338451d35@%3Cdev.beam.apache.org%3E
>
> -Rui
>
> On Tue, Mar 12, 2019 at 10:37 AM Reuven Lax <re...@google.com> wrote:
>
>> You are right that it was decided that a contributor can review a
>> committer's PR. I think the committer guide was never updated, and we
>> should do so.
>>
>> On Tue, Mar 12, 2019 at 10:28 AM Gleb Kanterov <gl...@spotify.com> wrote:
>>
>>> Before pressing merge button I was familiarizing myself with committer
>>> guide [1]. It's saying:
>>>
>>> > A committer (who is not the author of the code) should signal this
>>> either by GitHub “approval” or by a comment such as “Looks good to me!”
>>> (LGTM). Any committer can then merge the pull request. It is fine for a
>>> committer to self-merge if another committer has reviewed the code and
>>> approved it, just be sure to be explicit about whose job it is!
>>>
>>> As I understand it, it's saying that the reviewer should be Beam
>>> Committer. However, I remember from my personal experience and reading "An
>>> approach to community building from Apache Beam" [2] that
>>>
>>> > either the reviewer or the author be a committer
>>>
>>> I'm wondering if we could rephrase our Commiter Guide a bit to make it
>>> clear.
>>>
>>> [1]: https://beam.apache.org/contribute/committer-guide/
>>> [2]:
>>> https://blogs.apache.org/comdev/entry/an-approach-to-community-building
>>>
>>>
Re: Always get to LGTM in Committer Guide
Posted by Rui Wang <ru...@google.com>.
Yes. I think we changed the policy in [1].
Quote from the thread:
(1) At least one committer is involved with the code review, as either a
reviewer or as the author
(2) A contributor has approved the change
prior to merging any change.
This changes our policy from its current requirement that at least one
committer *who is not the author* has approved the change prior to merging.
We believe that changing this process will improve code review throughput,
reduce committer load, and engage more of the community in the code review
process.
[1]:
https://lists.apache.org/thread.html/34a10b9d4d0b3b1cc92132779ab505bcb4a759aa9ae40f3338451d35@%3Cdev.beam.apache.org%3E
-Rui
On Tue, Mar 12, 2019 at 10:37 AM Reuven Lax <re...@google.com> wrote:
> You are right that it was decided that a contributor can review a
> committer's PR. I think the committer guide was never updated, and we
> should do so.
>
> On Tue, Mar 12, 2019 at 10:28 AM Gleb Kanterov <gl...@spotify.com> wrote:
>
>> Before pressing merge button I was familiarizing myself with committer
>> guide [1]. It's saying:
>>
>> > A committer (who is not the author of the code) should signal this
>> either by GitHub “approval” or by a comment such as “Looks good to me!”
>> (LGTM). Any committer can then merge the pull request. It is fine for a
>> committer to self-merge if another committer has reviewed the code and
>> approved it, just be sure to be explicit about whose job it is!
>>
>> As I understand it, it's saying that the reviewer should be Beam
>> Committer. However, I remember from my personal experience and reading "An
>> approach to community building from Apache Beam" [2] that
>>
>> > either the reviewer or the author be a committer
>>
>> I'm wondering if we could rephrase our Commiter Guide a bit to make it
>> clear.
>>
>> [1]: https://beam.apache.org/contribute/committer-guide/
>> [2]:
>> https://blogs.apache.org/comdev/entry/an-approach-to-community-building
>>
>>
Re: Always get to LGTM in Committer Guide
Posted by Reuven Lax <re...@google.com>.
You are right that it was decided that a contributor can review a
committer's PR. I think the committer guide was never updated, and we
should do so.
On Tue, Mar 12, 2019 at 10:28 AM Gleb Kanterov <gl...@spotify.com> wrote:
> Before pressing merge button I was familiarizing myself with committer
> guide [1]. It's saying:
>
> > A committer (who is not the author of the code) should signal this
> either by GitHub “approval” or by a comment such as “Looks good to me!”
> (LGTM). Any committer can then merge the pull request. It is fine for a
> committer to self-merge if another committer has reviewed the code and
> approved it, just be sure to be explicit about whose job it is!
>
> As I understand it, it's saying that the reviewer should be Beam
> Committer. However, I remember from my personal experience and reading "An
> approach to community building from Apache Beam" [2] that
>
> > either the reviewer or the author be a committer
>
> I'm wondering if we could rephrase our Commiter Guide a bit to make it
> clear.
>
> [1]: https://beam.apache.org/contribute/committer-guide/
> [2]:
> https://blogs.apache.org/comdev/entry/an-approach-to-community-building
>
>