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 <bat...@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 <apill...@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 <k...@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 <mig...@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....@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 <echauc...@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 <k...@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 <rfern...@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 <bat...@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 <bat...@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 >>>>>> >>>>>> >>>>>> >>>>>
