You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Huygaa Batsaikhan <ba...@google.com> on 2018/06/27 00:17:22 UTC
[Design Proposal] Improving Beam code review
Hi, I've been looking into ways to improve Beam's code review process based
on previous discussions on dev list and summits, and I would like to
propose improvement ideas. Please take a look at:
https://s.apache.org/beam-code-review.
Main proposals suggested in the doc are:
1. Create a code review guideline document.
2. Build/setup code review tools and dashboards for Beam.
3. Collect metrics to monitor Beam's code review health.
Feel free to add comments in the doc. I am looking for all sorts of
suggestions including existing code review guidelines, potential code
review tools etc.
Thanks so much,
Huygaa
Re: [Design Proposal] Improving Beam code review
Posted by Eugene Kirpichov <ki...@google.com>.
Thanks for driving this Huygaa! I'm excited to see this happen, this is
going to make a big difference in contributor and reviewer experience.
On Fri, Jun 29, 2018 at 5:29 PM Huygaa Batsaikhan <ba...@google.com> wrote:
> Thanks everyone who reviewed the doc and suggested good ideas. Here is a
> recap of the document and the conversation.
>
> Proposals gained major support:
>
> - Creating a *code review guideline* - Reuven is working on creating a
> code review guideline and best practices doc.
> - Code review *metrics and dashboard* - Capture the state of Beam code
> review in numbers and monitor it over time.
> - *Auto-assigning reviewers* - Support for OWNERS file, assigning
> secondary non-committer reviewers
> - *Reviewer load dashboard* - Prevents reviewers from getting
> overloaded
>
> Proposals worth pursuing with more consideration and research:
>
> - *Code review dashboard* - Reviewers mostly supported the idea.
> However, the support will depend on how good the tool is, and if it could
> support features useful such as "whose turn"
> - Reviewer availability *calendar* - creates transparency, however,
> adds additional integration to Beam.
>
> I will be out on vacation for 2-3 weeks from now. When I come back, I will
> continue this work and start experimenting with tools accessible.
>
>
> On Fri, Jun 29, 2018 at 11:30 AM Andrew Pilloud <ap...@google.com>
> wrote:
>
>> Auto-assigning PRs to committers seems like something that would be
>> really helpful. There was no shortage of volunteers to help review SQL
>> while Kenn is out. It seems like it would be pretty easy to build a review
>> assignment bot and have a list of committers willing to volunteer check in
>> somewhere. (Kubernetes has this, we might be able to use their bot.)
>>
>> Andrew
>>
>> On Thu, Jun 28, 2018 at 2:28 PM Kenneth Knowles <kl...@google.com> wrote:
>>
>>> The main limitation is that only members of the "apache" GitHub
>>> organization can be assigned to these fields. Otherwise they would be
>>> perfect for tracking both who is doing the review and whose turn it is to
>>> take action!
>>>
>>> I don't know how Github CODEOWNERS behaves if you put a non-member in it.
>>>
>>> Kenn
>>>
>>> On Thu, Jun 28, 2018 at 2:23 PM Mikhail Gryzykhin <mi...@google.com>
>>> wrote:
>>>
>>>> That's a good document.
>>>>
>>>> I have a general question:
>>>> Is there a reason why we do not assign reviewer/assignee/labels to PRs?
>>>> I see that we add @reviewer comments, but never actually assign reviewers.
>>>> Those are good tools that Github can use as filters for you.
>>>>
>>>> --Mikhail
>>>>
>>>> Have feedback <http://go/migryz-feedback>?
>>>>
>>>>
>>>> On Thu, Jun 28, 2018 at 1:46 PM Ahmet Altay <al...@google.com> wrote:
>>>>
>>>>> Thank you Huygaa. This document looks good to me. I think
>>>>> auto-assigning PRs could significantly help especially with first time
>>>>> contributors. It could also give us a chance to distribute reviews in a
>>>>> more balanced way across committers.
>>>>>
>>>>> On Thu, Jun 28, 2018 at 7:37 AM, Alexey Romanenko <
>>>>> aromanenko.dev@gmail.com> wrote:
>>>>>
>>>>>> Strongly agree with auto assigning code reviewers, I guess this is
>>>>>> one of the main issue for first-starters to whom address their PR.
>>>>>>
>>>>>> Also, I’m totally pro for having review style guide which
>>>>>> definitively should help to unify review process and make it more
>>>>>> transparent for all.
>>>>>>
>>>>>> Thanks to last efforts to reduce a number of open PRs, there are only
>>>>>> about 90 opened ones. I believe that most of them are “in progress” but
>>>>>> others are quite inactive. Perhaps, it would make sense to put some efforts
>>>>>> to review their status before they will be closed automatically by stale
>>>>>> bot.
>>>>>>
>>>>>> Alexey
>>>>>>
>>>>>>
>>>>>> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org>
>>>>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Thanks for that ! I left comments in the doc, mostly agreements and
>>>>>> also a comment about public communication.
>>>>>>
>>>>>> Etienne
>>>>>>
>>>>>> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>>>>>>
>>>>>> Thanks for writing this up! I especially like the idea of
>>>>>>
>>>>>> automatically assigning code reviewers, e.g. via
>>>>>>
>>>>>> https://help.github.com/articles/about-codeowners/
>>>>>>
>>>>>> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>>>>>>
>>>>>>
>>>>>> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>>>>>>
>>>>>>
>>>>>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>>>>>
>>>>>>
>>>>>> Kenn
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>>>>>>
>>>>>>
>>>>>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>>>>>
>>>>>>
>>>>>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>>>>>>
>>>>>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>>>>>>
>>>>>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>>>>>
>>>>>>
>>>>>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>>>>>
>>>>>>
>>>>>> r
>>>>>>
>>>>>>
>>>>>> [1] https://www.apache.org/foundation/policies/conduct.html
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>>>
>>>>>>
>>>>>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>>>>>
>>>>>>
>>>>>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>>>
>>>>>>
>>>>>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>>>>>>
>>>>>>
>>>>>> Main proposals suggested in the doc are:
>>>>>>
>>>>>>
>>>>>> Create a code review guideline document.
>>>>>>
>>>>>> Build/setup code review tools and dashboards for Beam.
>>>>>>
>>>>>> Collect metrics to monitor Beam's code review health.
>>>>>>
>>>>>>
>>>>>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>>>>>
>>>>>>
>>>>>> Thanks so much,
>>>>>>
>>>>>> Huygaa
>>>>>>
>>>>>>
>>>>>>
>>>>>
Re: [Design Proposal] Improving Beam code review
Posted by Huygaa Batsaikhan <ba...@google.com>.
Thanks everyone who reviewed the doc and suggested good ideas. Here is a
recap of the document and the conversation.
Proposals gained major support:
- Creating a *code review guideline* - Reuven is working on creating a
code review guideline and best practices doc.
- Code review *metrics and dashboard* - Capture the state of Beam code
review in numbers and monitor it over time.
- *Auto-assigning reviewers* - Support for OWNERS file, assigning
secondary non-committer reviewers
- *Reviewer load dashboard* - Prevents reviewers from getting overloaded
Proposals worth pursuing with more consideration and research:
- *Code review dashboard* - Reviewers mostly supported the idea.
However, the support will depend on how good the tool is, and if it could
support features useful such as "whose turn"
- Reviewer availability *calendar* - creates transparency, however, adds
additional integration to Beam.
I will be out on vacation for 2-3 weeks from now. When I come back, I will
continue this work and start experimenting with tools accessible.
On Fri, Jun 29, 2018 at 11:30 AM Andrew Pilloud <ap...@google.com> wrote:
> Auto-assigning PRs to committers seems like something that would be really
> helpful. There was no shortage of volunteers to help review SQL while Kenn
> is out. It seems like it would be pretty easy to build a review assignment
> bot and have a list of committers willing to volunteer check in somewhere.
> (Kubernetes has this, we might be able to use their bot.)
>
> Andrew
>
> On Thu, Jun 28, 2018 at 2:28 PM Kenneth Knowles <kl...@google.com> wrote:
>
>> The main limitation is that only members of the "apache" GitHub
>> organization can be assigned to these fields. Otherwise they would be
>> perfect for tracking both who is doing the review and whose turn it is to
>> take action!
>>
>> I don't know how Github CODEOWNERS behaves if you put a non-member in it.
>>
>> Kenn
>>
>> On Thu, Jun 28, 2018 at 2:23 PM Mikhail Gryzykhin <mi...@google.com>
>> wrote:
>>
>>> That's a good document.
>>>
>>> I have a general question:
>>> Is there a reason why we do not assign reviewer/assignee/labels to PRs?
>>> I see that we add @reviewer comments, but never actually assign reviewers.
>>> Those are good tools that Github can use as filters for you.
>>>
>>> --Mikhail
>>>
>>> Have feedback <http://go/migryz-feedback>?
>>>
>>>
>>> On Thu, Jun 28, 2018 at 1:46 PM Ahmet Altay <al...@google.com> wrote:
>>>
>>>> Thank you Huygaa. This document looks good to me. I think
>>>> auto-assigning PRs could significantly help especially with first time
>>>> contributors. It could also give us a chance to distribute reviews in a
>>>> more balanced way across committers.
>>>>
>>>> On Thu, Jun 28, 2018 at 7:37 AM, Alexey Romanenko <
>>>> aromanenko.dev@gmail.com> wrote:
>>>>
>>>>> Strongly agree with auto assigning code reviewers, I guess this is one
>>>>> of the main issue for first-starters to whom address their PR.
>>>>>
>>>>> Also, I’m totally pro for having review style guide which definitively
>>>>> should help to unify review process and make it more transparent for all.
>>>>>
>>>>> Thanks to last efforts to reduce a number of open PRs, there are only
>>>>> about 90 opened ones. I believe that most of them are “in progress” but
>>>>> others are quite inactive. Perhaps, it would make sense to put some efforts
>>>>> to review their status before they will be closed automatically by stale
>>>>> bot.
>>>>>
>>>>> Alexey
>>>>>
>>>>>
>>>>> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org>
>>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Thanks for that ! I left comments in the doc, mostly agreements and
>>>>> also a comment about public communication.
>>>>>
>>>>> Etienne
>>>>>
>>>>> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>>>>>
>>>>> Thanks for writing this up! I especially like the idea of
>>>>>
>>>>> automatically assigning code reviewers, e.g. via
>>>>>
>>>>> https://help.github.com/articles/about-codeowners/
>>>>>
>>>>> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>>>>>
>>>>>
>>>>> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>>>>>
>>>>>
>>>>> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>>>>>
>>>>>
>>>>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>>>>
>>>>>
>>>>> Kenn
>>>>>
>>>>>
>>>>>
>>>>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>>>>>
>>>>>
>>>>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>>>>
>>>>>
>>>>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>>>>>
>>>>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>>>>>
>>>>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>>>>
>>>>>
>>>>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>>>>
>>>>>
>>>>> r
>>>>>
>>>>>
>>>>> [1] https://www.apache.org/foundation/policies/conduct.html
>>>>>
>>>>>
>>>>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>>
>>>>>
>>>>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>>>>
>>>>>
>>>>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>>>>
>>>>>
>>>>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>>>>
>>>>>
>>>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>>
>>>>>
>>>>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>>>>>
>>>>>
>>>>> Main proposals suggested in the doc are:
>>>>>
>>>>>
>>>>> Create a code review guideline document.
>>>>>
>>>>> Build/setup code review tools and dashboards for Beam.
>>>>>
>>>>> Collect metrics to monitor Beam's code review health.
>>>>>
>>>>>
>>>>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>>>>
>>>>>
>>>>> Thanks so much,
>>>>>
>>>>> Huygaa
>>>>>
>>>>>
>>>>>
>>>>
Re: [Design Proposal] Improving Beam code review
Posted by Andrew Pilloud <ap...@google.com>.
Auto-assigning PRs to committers seems like something that would be really
helpful. There was no shortage of volunteers to help review SQL while Kenn
is out. It seems like it would be pretty easy to build a review assignment
bot and have a list of committers willing to volunteer check in somewhere.
(Kubernetes has this, we might be able to use their bot.)
Andrew
On Thu, Jun 28, 2018 at 2:28 PM Kenneth Knowles <kl...@google.com> wrote:
> The main limitation is that only members of the "apache" GitHub
> organization can be assigned to these fields. Otherwise they would be
> perfect for tracking both who is doing the review and whose turn it is to
> take action!
>
> I don't know how Github CODEOWNERS behaves if you put a non-member in it.
>
> Kenn
>
> On Thu, Jun 28, 2018 at 2:23 PM Mikhail Gryzykhin <mi...@google.com>
> wrote:
>
>> That's a good document.
>>
>> I have a general question:
>> Is there a reason why we do not assign reviewer/assignee/labels to PRs? I
>> see that we add @reviewer comments, but never actually assign reviewers.
>> Those are good tools that Github can use as filters for you.
>>
>> --Mikhail
>>
>> Have feedback <http://go/migryz-feedback>?
>>
>>
>> On Thu, Jun 28, 2018 at 1:46 PM Ahmet Altay <al...@google.com> wrote:
>>
>>> Thank you Huygaa. This document looks good to me. I think auto-assigning
>>> PRs could significantly help especially with first time contributors. It
>>> could also give us a chance to distribute reviews in a more balanced way
>>> across committers.
>>>
>>> On Thu, Jun 28, 2018 at 7:37 AM, Alexey Romanenko <
>>> aromanenko.dev@gmail.com> wrote:
>>>
>>>> Strongly agree with auto assigning code reviewers, I guess this is one
>>>> of the main issue for first-starters to whom address their PR.
>>>>
>>>> Also, I’m totally pro for having review style guide which definitively
>>>> should help to unify review process and make it more transparent for all.
>>>>
>>>> Thanks to last efforts to reduce a number of open PRs, there are only
>>>> about 90 opened ones. I believe that most of them are “in progress” but
>>>> others are quite inactive. Perhaps, it would make sense to put some efforts
>>>> to review their status before they will be closed automatically by stale
>>>> bot.
>>>>
>>>> Alexey
>>>>
>>>>
>>>> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Thanks for that ! I left comments in the doc, mostly agreements and
>>>> also a comment about public communication.
>>>>
>>>> Etienne
>>>>
>>>> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>>>>
>>>> Thanks for writing this up! I especially like the idea of
>>>>
>>>> automatically assigning code reviewers, e.g. via
>>>>
>>>> https://help.github.com/articles/about-codeowners/
>>>>
>>>> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>>>>
>>>>
>>>> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>>>>
>>>>
>>>> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>>>>
>>>>
>>>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>>>
>>>>
>>>> Kenn
>>>>
>>>>
>>>>
>>>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>>>>
>>>>
>>>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>>>
>>>>
>>>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>>>>
>>>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>>>>
>>>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>>>
>>>>
>>>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>>>
>>>>
>>>> r
>>>>
>>>>
>>>> [1] https://www.apache.org/foundation/policies/conduct.html
>>>>
>>>>
>>>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>
>>>>
>>>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>>>
>>>>
>>>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>>>
>>>>
>>>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>>>
>>>>
>>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>
>>>>
>>>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>>>>
>>>>
>>>> Main proposals suggested in the doc are:
>>>>
>>>>
>>>> Create a code review guideline document.
>>>>
>>>> Build/setup code review tools and dashboards for Beam.
>>>>
>>>> Collect metrics to monitor Beam's code review health.
>>>>
>>>>
>>>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>>>
>>>>
>>>> Thanks so much,
>>>>
>>>> Huygaa
>>>>
>>>>
>>>>
>>>
Re: [Design Proposal] Improving Beam code review
Posted by Kenneth Knowles <kl...@google.com>.
The main limitation is that only members of the "apache" GitHub
organization can be assigned to these fields. Otherwise they would be
perfect for tracking both who is doing the review and whose turn it is to
take action!
I don't know how Github CODEOWNERS behaves if you put a non-member in it.
Kenn
On Thu, Jun 28, 2018 at 2:23 PM Mikhail Gryzykhin <mi...@google.com> wrote:
> That's a good document.
>
> I have a general question:
> Is there a reason why we do not assign reviewer/assignee/labels to PRs? I
> see that we add @reviewer comments, but never actually assign reviewers.
> Those are good tools that Github can use as filters for you.
>
> --Mikhail
>
> Have feedback <http://go/migryz-feedback>?
>
>
> On Thu, Jun 28, 2018 at 1:46 PM Ahmet Altay <al...@google.com> wrote:
>
>> Thank you Huygaa. This document looks good to me. I think auto-assigning
>> PRs could significantly help especially with first time contributors. It
>> could also give us a chance to distribute reviews in a more balanced way
>> across committers.
>>
>> On Thu, Jun 28, 2018 at 7:37 AM, Alexey Romanenko <
>> aromanenko.dev@gmail.com> wrote:
>>
>>> Strongly agree with auto assigning code reviewers, I guess this is one
>>> of the main issue for first-starters to whom address their PR.
>>>
>>> Also, I’m totally pro for having review style guide which definitively
>>> should help to unify review process and make it more transparent for all.
>>>
>>> Thanks to last efforts to reduce a number of open PRs, there are only
>>> about 90 opened ones. I believe that most of them are “in progress” but
>>> others are quite inactive. Perhaps, it would make sense to put some efforts
>>> to review their status before they will be closed automatically by stale
>>> bot.
>>>
>>> Alexey
>>>
>>>
>>> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org> wrote:
>>>
>>> Hi,
>>>
>>> Thanks for that ! I left comments in the doc, mostly agreements and also
>>> a comment about public communication.
>>>
>>> Etienne
>>>
>>> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>>>
>>> Thanks for writing this up! I especially like the idea of
>>>
>>> automatically assigning code reviewers, e.g. via
>>>
>>> https://help.github.com/articles/about-codeowners/
>>>
>>> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>>>
>>>
>>> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>>>
>>>
>>> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>>>
>>>
>>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>>
>>>
>>> Kenn
>>>
>>>
>>>
>>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>>>
>>>
>>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>>
>>>
>>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>>>
>>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>>>
>>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>>
>>>
>>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>>
>>>
>>> r
>>>
>>>
>>> [1] https://www.apache.org/foundation/policies/conduct.html
>>>
>>>
>>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>
>>>
>>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>>
>>>
>>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>>
>>>
>>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>>
>>>
>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>
>>>
>>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>>>
>>>
>>> Main proposals suggested in the doc are:
>>>
>>>
>>> Create a code review guideline document.
>>>
>>> Build/setup code review tools and dashboards for Beam.
>>>
>>> Collect metrics to monitor Beam's code review health.
>>>
>>>
>>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>>
>>>
>>> Thanks so much,
>>>
>>> Huygaa
>>>
>>>
>>>
>>
Re: [Design Proposal] Improving Beam code review
Posted by Mikhail Gryzykhin <mi...@google.com>.
That's a good document.
I have a general question:
Is there a reason why we do not assign reviewer/assignee/labels to PRs? I
see that we add @reviewer comments, but never actually assign reviewers.
Those are good tools that Github can use as filters for you.
--Mikhail
Have feedback <http://go/migryz-feedback>?
On Thu, Jun 28, 2018 at 1:46 PM Ahmet Altay <al...@google.com> wrote:
> Thank you Huygaa. This document looks good to me. I think auto-assigning
> PRs could significantly help especially with first time contributors. It
> could also give us a chance to distribute reviews in a more balanced way
> across committers.
>
> On Thu, Jun 28, 2018 at 7:37 AM, Alexey Romanenko <
> aromanenko.dev@gmail.com> wrote:
>
>> Strongly agree with auto assigning code reviewers, I guess this is one of
>> the main issue for first-starters to whom address their PR.
>>
>> Also, I’m totally pro for having review style guide which definitively
>> should help to unify review process and make it more transparent for all.
>>
>> Thanks to last efforts to reduce a number of open PRs, there are only
>> about 90 opened ones. I believe that most of them are “in progress” but
>> others are quite inactive. Perhaps, it would make sense to put some efforts
>> to review their status before they will be closed automatically by stale
>> bot.
>>
>> Alexey
>>
>>
>> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org> wrote:
>>
>> Hi,
>>
>> Thanks for that ! I left comments in the doc, mostly agreements and also
>> a comment about public communication.
>>
>> Etienne
>>
>> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>>
>> Thanks for writing this up! I especially like the idea of
>>
>> automatically assigning code reviewers, e.g. via
>>
>> https://help.github.com/articles/about-codeowners/
>>
>> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>>
>>
>> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>>
>>
>> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>>
>>
>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>
>>
>> Kenn
>>
>>
>>
>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>>
>>
>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>
>>
>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>>
>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>>
>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>
>>
>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>
>>
>> r
>>
>>
>> [1] https://www.apache.org/foundation/policies/conduct.html
>>
>>
>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>
>>
>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>
>>
>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>
>>
>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>
>>
>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>
>>
>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>>
>>
>> Main proposals suggested in the doc are:
>>
>>
>> Create a code review guideline document.
>>
>> Build/setup code review tools and dashboards for Beam.
>>
>> Collect metrics to monitor Beam's code review health.
>>
>>
>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>
>>
>> Thanks so much,
>>
>> Huygaa
>>
>>
>>
>
Re: [Design Proposal] Improving Beam code review
Posted by Ahmet Altay <al...@google.com>.
Thank you Huygaa. This document looks good to me. I think auto-assigning
PRs could significantly help especially with first time contributors. It
could also give us a chance to distribute reviews in a more balanced way
across committers.
On Thu, Jun 28, 2018 at 7:37 AM, Alexey Romanenko <ar...@gmail.com>
wrote:
> Strongly agree with auto assigning code reviewers, I guess this is one of
> the main issue for first-starters to whom address their PR.
>
> Also, I’m totally pro for having review style guide which definitively
> should help to unify review process and make it more transparent for all.
>
> Thanks to last efforts to reduce a number of open PRs, there are only
> about 90 opened ones. I believe that most of them are “in progress” but
> others are quite inactive. Perhaps, it would make sense to put some efforts
> to review their status before they will be closed automatically by stale
> bot.
>
> Alexey
>
>
> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org> wrote:
>
> Hi,
>
> Thanks for that ! I left comments in the doc, mostly agreements and also a
> comment about public communication.
>
> Etienne
>
> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>
> Thanks for writing this up! I especially like the idea of
>
> automatically assigning code reviewers, e.g. via
>
> https://help.github.com/articles/about-codeowners/
>
> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>
>
> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>
>
> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>
>
> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>
>
> Kenn
>
>
>
> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>
>
> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>
>
> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>
> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>
> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>
>
> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>
>
> r
>
>
> [1] https://www.apache.org/foundation/policies/conduct.html
>
>
> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>
>
> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>
>
> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>
>
> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>
>
> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>
>
> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>
>
> Main proposals suggested in the doc are:
>
>
> Create a code review guideline document.
>
> Build/setup code review tools and dashboards for Beam.
>
> Collect metrics to monitor Beam's code review health.
>
>
> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>
>
> Thanks so much,
>
> Huygaa
>
>
>
Re: [Design Proposal] Improving Beam code review
Posted by Alexey Romanenko <ar...@gmail.com>.
Strongly agree with auto assigning code reviewers, I guess this is one of the main issue for first-starters to whom address their PR.
Also, I’m totally pro for having review style guide which definitively should help to unify review process and make it more transparent for all.
Thanks to last efforts to reduce a number of open PRs, there are only about 90 opened ones. I believe that most of them are “in progress” but others are quite inactive. Perhaps, it would make sense to put some efforts to review their status before they will be closed automatically by stale bot.
Alexey
> On 28 Jun 2018, at 10:24, Etienne Chauchot <ec...@apache.org> wrote:
>
> Hi,
>
> Thanks for that ! I left comments in the doc, mostly agreements and also a comment about public communication.
>
> Etienne
>
> Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
>> Thanks for writing this up! I especially like the idea of
>> automatically assigning code reviewers, e.g. via
>> https://help.github.com/articles/about-codeowners/ <https://help.github.com/articles/about-codeowners/>
>> On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <scott@apache.org <ma...@apache.org>> wrote:
>>
>>
>> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>>
>> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <klk@google.com <ma...@google.com>> wrote:
>>
>>
>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>
>> Kenn
>>
>>
>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rfernand@google.com <ma...@google.com>> wrote:
>>
>>
>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>
>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>
>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>
>> r
>>
>> [1] https://www.apache.org/foundation/policies/conduct.html <https://www.apache.org/foundation/policies/conduct.html>
>>
>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <batbat@google.com <ma...@google.com>> wrote:
>>
>>
>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>
>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <relax@google.com <ma...@google.com>> wrote:
>>
>>
>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>
>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <batbat@google.com <ma...@google.com>> wrote:
>>
>>
>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review <https://s.apache.org/beam-code-review>.
>>
>> Main proposals suggested in the doc are:
>>
>> Create a code review guideline document.
>> Build/setup code review tools and dashboards for Beam.
>> Collect metrics to monitor Beam's code review health.
>>
>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>
>> Thanks so much,
>> Huygaa
Re: [Design Proposal] Improving Beam code review
Posted by Etienne Chauchot <ec...@apache.org>.
Hi,
Thanks for that ! I left comments in the doc, mostly agreements and also a comment about public communication.
Etienne
Le mercredi 27 juin 2018 à 15:29 -0700, Robert Bradshaw a écrit :
> Thanks for writing this up! I especially like the idea ofautomatically assigning code reviewers, e.g. viahttps://help.
> github.com/articles/about-codeowners/On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>
> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>
> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers &
> stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and
> 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other
> reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
> Kenn
>
> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>
> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a
> very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on
> this).I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go,
> etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time
> working with the contributor to make sure together they can introduce great code that is consistent with the codebase
> (so they can focus on functionality and scale discussions, not style, which is published).I think setting review
> expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are
> paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the
> Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to
> say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time
> sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't
> commit to reviewing this by Friday, I suggest another person.", for example.
> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we
> can all continue to grow the project as fast as we want.
> r
> [1] https://www.apache.org/foundation/policies/conduct.html
> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>
> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>
> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now
> drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>
> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and
> summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
> Main proposals suggested in the doc are:
> Create a code review guideline document.Build/setup code review tools and dashboards for Beam.Collect metrics to
> monitor Beam's code review health.
> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review
> guidelines, potential code review tools etc.
> Thanks so much,Huygaa
Re: [Design Proposal] Improving Beam code review
Posted by Robert Bradshaw <ro...@google.com>.
Thanks for writing this up! I especially like the idea of
automatically assigning code reviewers, e.g. via
https://help.github.com/articles/about-codeowners/
On Wed, Jun 27, 2018 at 11:10 AM Scott Wegner <sc...@apache.org> wrote:
>
> Thanks for putting together this proposal Huygaa. Overall looks good to me; I added some comments in the doc.
>
> On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
>>
>> Does Kubernetes keep up with their backlog? We were hovering around 100 before our recent addition of committers & stalebot, and now around 80. I can imagine their 1000 open PRs might be an OK steady state; they have some 6 month and 2 month PRs but the overall distribution might be sort of like ours. Is the data in a table somewhere? Couple other reference points: Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>>
>> Kenn
>>
>>
>> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com> wrote:
>>>
>>> I did a quick pass on the doc and left minor comments, thanks! I have some feedback and thoughts:
>>>
>>> For metrics and tools, there ought to be mature OSS projects out there we can learn from. I believe Kubernetes has a very healthy practice, it'd be ideal to learn from them. +Griselda Cuevas can connect you (and people working on this).
>>> I really like the idea of a style guide (which can evolve) for the various areas - presumably Java, Python, Go, etc. have their own. The reason I like it is because reviews become easier -- the reviewer will have an easier time working with the contributor to make sure together they can introduce great code that is consistent with the codebase (so they can focus on functionality and scale discussions, not style, which is published).
>>> I think setting review expectations is hard. Many of us in the community have various degrees of time devoted to development - some of us are paid to work on Beam full time, others part time, others are gifting their time and talent. I find inspiration in the Apache Code of Conduct [1] to instead empower people to communicate clearly. A company or a developer may choose to say "This is what you can expect from me", and say, opt-in to email reminders and such. And when something is time sensitive, we should trust reviewers to be Apache-y and do a micro version of "Step down consderately" -- "I can't commit to reviewing this by Friday, I suggest another person.", for example.
>>>
>>> I think at the end of the day we all need to eliminate guesswork and promote the healthiest communication we can so we can all continue to grow the project as fast as we want.
>>>
>>> r
>>>
>>> [1] https://www.apache.org/foundation/policies/conduct.html
>>>
>>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>
>>>> Reuven, that's great. In this thread, we can continue discussing the usage of review tools, dashboards, and metrics.
>>>>
>>>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>>>>
>>>>> So I suggested a while ago that we create a code-review guidelines doc, and in fact I was coincidentally just now drafting up a proposal doc. I'll share my proposal doc with the dev list soon.
>>>>>
>>>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
>>>>>>
>>>>>> Hi, I've been looking into ways to improve Beam's code review process based on previous discussions on dev list and summits, and I would like to propose improvement ideas. Please take a look at: https://s.apache.org/beam-code-review.
>>>>>>
>>>>>> Main proposals suggested in the doc are:
>>>>>>
>>>>>> Create a code review guideline document.
>>>>>> Build/setup code review tools and dashboards for Beam.
>>>>>> Collect metrics to monitor Beam's code review health.
>>>>>>
>>>>>> Feel free to add comments in the doc. I am looking for all sorts of suggestions including existing code review guidelines, potential code review tools etc.
>>>>>>
>>>>>> Thanks so much,
>>>>>> Huygaa
Re: [Design Proposal] Improving Beam code review
Posted by Scott Wegner <sc...@apache.org>.
Thanks for putting together this proposal Huygaa. Overall looks good to me;
I added some comments in the doc.
On Tue, Jun 26, 2018 at 7:44 PM Kenneth Knowles <kl...@google.com> wrote:
> Does Kubernetes keep up with their backlog? We were hovering around 100
> before our recent addition of committers & stalebot, and now around 80. I
> can imagine their 1000 open PRs might be an OK steady state; they have some
> 6 month and 2 month PRs but the overall distribution might be sort of like
> ours. Is the data in a table somewhere? Couple other reference points:
> Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
>
> Kenn
>
>
> On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com>
> wrote:
>
>> I did a quick pass on the doc and left minor comments, thanks! I have
>> some feedback and thoughts:
>>
>> - For metrics and tools, there ought to be mature OSS projects out
>> there we can learn from. I believe Kubernetes has a very healthy practice,
>> it'd be ideal to learn from them. +Griselda Cuevas <gr...@google.com> can
>> connect you (and people working on this).
>> - I really like the idea of a style guide (which can evolve) for the
>> various areas - presumably Java, Python, Go, etc. have their own. The
>> reason I like it is because reviews become easier -- the reviewer will have
>> an easier time working with the contributor to make sure together they can
>> introduce great code that is consistent with the codebase (so they can
>> focus on functionality and scale discussions, not style, which is
>> published).
>> - I think setting review expectations is hard. Many of us in the
>> community have various degrees of time devoted to development - some of us
>> are paid to work on Beam full time, others part time, others are gifting
>> their time and talent. I find inspiration in the Apache Code of Conduct [1]
>> to instead empower people to communicate clearly. A company or a developer
>> may choose to say "This is what you can expect from me", and say, opt-in to
>> email reminders and such. And when something is time sensitive, we should
>> trust reviewers to be Apache-y and do a micro version of "*Step down
>> consderately*" -- "I can't commit to reviewing this by Friday, I
>> suggest another person.", for example.
>>
>> I think at the end of the day we all need to eliminate guesswork and
>> promote the healthiest communication we can so we can all continue to grow
>> the project as fast as we want.
>>
>> r
>>
>> [1] https://www.apache.org/foundation/policies/conduct.html
>>
>> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com>
>> wrote:
>>
>>> Reuven, that's great. In this thread, we can continue discussing the
>>> usage of review tools, dashboards, and metrics.
>>>
>>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>>
>>>> So I suggested a while ago that we create a code-review guidelines doc,
>>>> and in fact I was coincidentally just now drafting up a proposal doc. I'll
>>>> share my proposal doc with the dev list soon.
>>>>
>>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com>
>>>> wrote:
>>>>
>>>>> Hi, I've been looking into ways to improve Beam's code review process
>>>>> based on previous discussions on dev list and summits, and I would like to
>>>>> propose improvement ideas. Please take a look at:
>>>>> https://s.apache.org/beam-code-review.
>>>>>
>>>>> Main proposals suggested in the doc are:
>>>>>
>>>>> 1. Create a code review guideline document.
>>>>> 2. Build/setup code review tools and dashboards for Beam.
>>>>> 3. Collect metrics to monitor Beam's code review health.
>>>>>
>>>>> Feel free to add comments in the doc. I am looking for all sorts of
>>>>> suggestions including existing code review guidelines, potential code
>>>>> review tools etc.
>>>>>
>>>>> Thanks so much,
>>>>> Huygaa
>>>>>
>>>>
Re: [Design Proposal] Improving Beam code review
Posted by Kenneth Knowles <kl...@google.com>.
Does Kubernetes keep up with their backlog? We were hovering around 100
before our recent addition of committers & stalebot, and now around 80. I
can imagine their 1000 open PRs might be an OK steady state; they have some
6 month and 2 month PRs but the overall distribution might be sort of like
ours. Is the data in a table somewhere? Couple other reference points:
Spark has ~500, Flink ~400, Storm ~150, Rust ~150.
Kenn
On Tue, Jun 26, 2018 at 6:35 PM Rafael Fernandez <rf...@google.com>
wrote:
> I did a quick pass on the doc and left minor comments, thanks! I have some
> feedback and thoughts:
>
> - For metrics and tools, there ought to be mature OSS projects out
> there we can learn from. I believe Kubernetes has a very healthy practice,
> it'd be ideal to learn from them. +Griselda Cuevas <gr...@google.com> can
> connect you (and people working on this).
> - I really like the idea of a style guide (which can evolve) for the
> various areas - presumably Java, Python, Go, etc. have their own. The
> reason I like it is because reviews become easier -- the reviewer will have
> an easier time working with the contributor to make sure together they can
> introduce great code that is consistent with the codebase (so they can
> focus on functionality and scale discussions, not style, which is
> published).
> - I think setting review expectations is hard. Many of us in the
> community have various degrees of time devoted to development - some of us
> are paid to work on Beam full time, others part time, others are gifting
> their time and talent. I find inspiration in the Apache Code of Conduct [1]
> to instead empower people to communicate clearly. A company or a developer
> may choose to say "This is what you can expect from me", and say, opt-in to
> email reminders and such. And when something is time sensitive, we should
> trust reviewers to be Apache-y and do a micro version of "*Step down
> consderately*" -- "I can't commit to reviewing this by Friday, I
> suggest another person.", for example.
>
> I think at the end of the day we all need to eliminate guesswork and
> promote the healthiest communication we can so we can all continue to grow
> the project as fast as we want.
>
> r
>
> [1] https://www.apache.org/foundation/policies/conduct.html
>
> On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com>
> wrote:
>
>> Reuven, that's great. In this thread, we can continue discussing the
>> usage of review tools, dashboards, and metrics.
>>
>> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>>
>>> So I suggested a while ago that we create a code-review guidelines doc,
>>> and in fact I was coincidentally just now drafting up a proposal doc. I'll
>>> share my proposal doc with the dev list soon.
>>>
>>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com>
>>> wrote:
>>>
>>>> Hi, I've been looking into ways to improve Beam's code review process
>>>> based on previous discussions on dev list and summits, and I would like to
>>>> propose improvement ideas. Please take a look at:
>>>> https://s.apache.org/beam-code-review.
>>>>
>>>> Main proposals suggested in the doc are:
>>>>
>>>> 1. Create a code review guideline document.
>>>> 2. Build/setup code review tools and dashboards for Beam.
>>>> 3. Collect metrics to monitor Beam's code review health.
>>>>
>>>> Feel free to add comments in the doc. I am looking for all sorts of
>>>> suggestions including existing code review guidelines, potential code
>>>> review tools etc.
>>>>
>>>> Thanks so much,
>>>> Huygaa
>>>>
>>>
Re: [Design Proposal] Improving Beam code review
Posted by Rafael Fernandez <rf...@google.com>.
I did a quick pass on the doc and left minor comments, thanks! I have some
feedback and thoughts:
- For metrics and tools, there ought to be mature OSS projects out there
we can learn from. I believe Kubernetes has a very healthy practice, it'd
be ideal to learn from them. +Griselda Cuevas <gr...@google.com> can
connect you (and people working on this).
- I really like the idea of a style guide (which can evolve) for the
various areas - presumably Java, Python, Go, etc. have their own. The
reason I like it is because reviews become easier -- the reviewer will have
an easier time working with the contributor to make sure together they can
introduce great code that is consistent with the codebase (so they can
focus on functionality and scale discussions, not style, which is
published).
- I think setting review expectations is hard. Many of us in the
community have various degrees of time devoted to development - some of us
are paid to work on Beam full time, others part time, others are gifting
their time and talent. I find inspiration in the Apache Code of Conduct [1]
to instead empower people to communicate clearly. A company or a developer
may choose to say "This is what you can expect from me", and say, opt-in to
email reminders and such. And when something is time sensitive, we should
trust reviewers to be Apache-y and do a micro version of "*Step down
consderately*" -- "I can't commit to reviewing this by Friday, I suggest
another person.", for example.
I think at the end of the day we all need to eliminate guesswork and
promote the healthiest communication we can so we can all continue to grow
the project as fast as we want.
r
[1] https://www.apache.org/foundation/policies/conduct.html
On Tue, Jun 26, 2018 at 5:48 PM Huygaa Batsaikhan <ba...@google.com> wrote:
> Reuven, that's great. In this thread, we can continue discussing the usage
> of review tools, dashboards, and metrics.
>
> On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
>
>> So I suggested a while ago that we create a code-review guidelines doc,
>> and in fact I was coincidentally just now drafting up a proposal doc. I'll
>> share my proposal doc with the dev list soon.
>>
>> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com>
>> wrote:
>>
>>> Hi, I've been looking into ways to improve Beam's code review process
>>> based on previous discussions on dev list and summits, and I would like to
>>> propose improvement ideas. Please take a look at:
>>> https://s.apache.org/beam-code-review.
>>>
>>> Main proposals suggested in the doc are:
>>>
>>> 1. Create a code review guideline document.
>>> 2. Build/setup code review tools and dashboards for Beam.
>>> 3. Collect metrics to monitor Beam's code review health.
>>>
>>> Feel free to add comments in the doc. I am looking for all sorts of
>>> suggestions including existing code review guidelines, potential code
>>> review tools etc.
>>>
>>> Thanks so much,
>>> Huygaa
>>>
>>
Re: [Design Proposal] Improving Beam code review
Posted by Huygaa Batsaikhan <ba...@google.com>.
Reuven, that's great. In this thread, we can continue discussing the usage
of review tools, dashboards, and metrics.
On Tue, Jun 26, 2018 at 5:27 PM Reuven Lax <re...@google.com> wrote:
> So I suggested a while ago that we create a code-review guidelines doc,
> and in fact I was coincidentally just now drafting up a proposal doc. I'll
> share my proposal doc with the dev list soon.
>
> On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com>
> wrote:
>
>> Hi, I've been looking into ways to improve Beam's code review process
>> based on previous discussions on dev list and summits, and I would like to
>> propose improvement ideas. Please take a look at:
>> https://s.apache.org/beam-code-review.
>>
>> Main proposals suggested in the doc are:
>>
>> 1. Create a code review guideline document.
>> 2. Build/setup code review tools and dashboards for Beam.
>> 3. Collect metrics to monitor Beam's code review health.
>>
>> Feel free to add comments in the doc. I am looking for all sorts of
>> suggestions including existing code review guidelines, potential code
>> review tools etc.
>>
>> Thanks so much,
>> Huygaa
>>
>
Re: [Design Proposal] Improving Beam code review
Posted by Reuven Lax <re...@google.com>.
So I suggested a while ago that we create a code-review guidelines doc, and
in fact I was coincidentally just now drafting up a proposal doc. I'll
share my proposal doc with the dev list soon.
On Tue, Jun 26, 2018 at 5:18 PM Huygaa Batsaikhan <ba...@google.com> wrote:
> Hi, I've been looking into ways to improve Beam's code review process
> based on previous discussions on dev list and summits, and I would like to
> propose improvement ideas. Please take a look at:
> https://s.apache.org/beam-code-review.
>
> Main proposals suggested in the doc are:
>
> 1. Create a code review guideline document.
> 2. Build/setup code review tools and dashboards for Beam.
> 3. Collect metrics to monitor Beam's code review health.
>
> Feel free to add comments in the doc. I am looking for all sorts of
> suggestions including existing code review guidelines, potential code
> review tools etc.
>
> Thanks so much,
> Huygaa
>
Re: [Design Proposal] Improving Beam code review
Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
Gonna take a look.
Thanks !!
Regards
JB
Le 27 juin 2018 à 08:18, à 08:18, Huygaa Batsaikhan <ba...@google.com> a écrit:
>Hi, I've been looking into ways to improve Beam's code review process
>based
>on previous discussions on dev list and summits, and I would like to
>propose improvement ideas. Please take a look at:
>https://s.apache.org/beam-code-review.
>
>Main proposals suggested in the doc are:
>
> 1. Create a code review guideline document.
> 2. Build/setup code review tools and dashboards for Beam.
> 3. Collect metrics to monitor Beam's code review health.
>
>Feel free to add comments in the doc. I am looking for all sorts of
>suggestions including existing code review guidelines, potential code
>review tools etc.
>
>Thanks so much,
>Huygaa