This sounds like a good plan. Did we want to rename the CODEOWNERS file to disable github's mass adding of reviewers while we figure this out?
Andrew On Mon, Jul 16, 2018 at 10:20 AM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > +1 > > Le 16 juil. 2018, à 19:17, Holden Karau <holden.ka...@gmail.com> a écrit: >> >> Ok if no one objects I'll create the INFRA ticket after OSCON and we can >> test it for a week and decide if it helps or hinders. >> >> On Mon, Jul 16, 2018, 7:12 PM Jean-Baptiste Onofré < j...@nanthrax.net> >> wrote: >> >>> Agree to test it for a week. >>> >>> Regards >>> JB >>> Le 16 juil. 2018, à 18:59, Holden Karau < holden.ka...@gmail.com> a >>> écrit: >>>> >>>> Would folks be OK with me asking infra to turn on blame based >>>> suggestions for Beam and trying it out for a week? >>>> >>>> On Mon, Jul 16, 2018, 6:53 PM Rafael Fernandez < rfern...@google.com> >>>> wrote: >>>> >>>>> +1 using blame -- nifty :) >>>>> >>>>> On Mon, Jul 16, 2018 at 2:31 AM Huygaa Batsaikhan < bat...@google.com> >>>>> wrote: >>>>> >>>>>> +1. This is great. >>>>>> >>>>>> On Sat, Jul 14, 2018 at 7:44 AM Udi Meiri < eh...@google.com> wrote: >>>>>> >>>>>>> Mention bot looks cool, as it tries to guess the reviewer using >>>>>>> blame. >>>>>>> I've written a quick and dirty script that uses only CODEOWNERS. >>>>>>> >>>>>>> Its output looks like: >>>>>>> $ python suggest_reviewers.py --pr 5940 >>>>>>> INFO:root:Selected reviewer @lukecwik for: >>>>>>> /runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformMatchers.java >>>>>>> (path_pattern: /runners/core-construction-java*) >>>>>>> INFO:root:Selected reviewer @lukecwik for: >>>>>>> /runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/SplittableParDoNaiveBounded.java >>>>>>> (path_pattern: /runners/core-construction-java*) >>>>>>> INFO:root:Selected reviewer @echauchot for: >>>>>>> /runners/core-java/src/main/java/org/apache/beam/runners/core/SplittableParDoViaKeyedWorkItems.java >>>>>>> (path_pattern: /runners/core-java*) >>>>>>> INFO:root:Selected reviewer @lukecwik for: >>>>>>> /runners/flink/build.gradle (path_pattern: */build.gradle*) >>>>>>> INFO:root:Selected reviewer @lukecwik for: >>>>>>> /runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkTransformOverrides.java >>>>>>> (path_pattern: *.java) >>>>>>> INFO:root:Selected reviewer @pabloem for: >>>>>>> /runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java >>>>>>> (path_pattern: /runners/google-cloud-dataflow-java*) >>>>>>> INFO:root:Selected reviewer @lukecwik for: >>>>>>> /sdks/java/core/src/test/java/org/apache/beam/sdk/transforms/SplittableDoFnTest.java >>>>>>> (path_pattern: /sdks/java/core*) >>>>>>> Suggested reviewers: @echauchot, @lukecwik, @pabloem >>>>>>> >>>>>>> Script is in: https://github.com/apache/beam/pull/5951 >>>>>>> >>>>>>> >>>>>>> What does the community think? Do you prefer blame-based or >>>>>>> rules-based reviewer suggestions? >>>>>>> >>>>>>> On Fri, Jul 13, 2018 at 11:13 AM Holden Karau < hol...@pigscanfly.ca> >>>>>>> wrote: >>>>>>> >>>>>>>> I'm looking at something similar in the Spark project, and while >>>>>>>> it's now archived by FB it seems like something like >>>>>>>> https://github.com/facebookarchive/mention-bot might do what we >>>>>>>> want. I'm going to spin up a version on my K8 cluster and see if I can >>>>>>>> ask >>>>>>>> infra to add a webhook and if it works for Spark we could ask INFRA to >>>>>>>> add >>>>>>>> a second webhook for Beam. (Or if the Beam folks are more interested in >>>>>>>> experimenting I can do Beam first as a smaller project and roll with >>>>>>>> that). >>>>>>>> >>>>>>>> Let me know :) >>>>>>>> >>>>>>>> On Fri, Jul 13, 2018 at 10:53 AM, Eugene Kirpichov < >>>>>>>> kirpic...@google.com> wrote: >>>>>>>> >>>>>>>>> Sounds reasonable for now, thanks! >>>>>>>>> It's unfortunate that Github's CODEOWNERS feature appears to be >>>>>>>>> effectively unusable for Beam but I'd hope that Github might pay >>>>>>>>> attention >>>>>>>>> and fix things if we submit feedback, with us being one of the most >>>>>>>>> active >>>>>>>>> Apache projects - did anyone do this yet / planning to? >>>>>>>>> >>>>>>>>> On Fri, Jul 13, 2018 at 10:23 AM Udi Meiri < eh...@google.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> While I like the idea of having a CODEOWNERS file, the Github >>>>>>>>>> implementation is lacking: >>>>>>>>>> 1. Reviewers are automatically assigned at each push. >>>>>>>>>> 2. Reviewer assignment can be excessive (e.g. 5 reviewers in >>>>>>>>>> Eugene's PR 5940). >>>>>>>>>> 3. Non-committers aren't assigned as reviewers. >>>>>>>>>> 4. Non-committers can't change the list of reviewers. >>>>>>>>>> >>>>>>>>>> I propose renaming the file to disable the auto-reviewer >>>>>>>>>> assignment feature. >>>>>>>>>> In its place I'll add a script that suggests reviewers. >>>>>>>>>> >>>>>>>>>> On Fri, Jul 13, 2018 at 9:09 AM Udi Meiri < eh...@google.com> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Etienne, >>>>>>>>>>> >>>>>>>>>>> Yes you could be as precise as you want. The paths I listed are >>>>>>>>>>> just suggestions. :) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Fri, Jul 13, 2018 at 1:12 AM Jean-Baptiste Onofré < >>>>>>>>>>> j...@nanthrax.net> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi, >>>>>>>>>>>> >>>>>>>>>>>> I think it's already do-able just providing the expected path. >>>>>>>>>>>> >>>>>>>>>>>> It's a good idea especially for the core. >>>>>>>>>>>> >>>>>>>>>>>> Regards >>>>>>>>>>>> JB >>>>>>>>>>>> >>>>>>>>>>>> On 13/07/2018 09:51, Etienne Chauchot wrote: >>>>>>>>>>>> > Hi Udi, >>>>>>>>>>>> > >>>>>>>>>>>> > I also have a question, related to what Eugene asked : I see >>>>>>>>>>>> that the >>>>>>>>>>>> > code paths are the ones of the modules. Can we be more >>>>>>>>>>>> precise than that >>>>>>>>>>>> > to assign reviewers ? As an example, I added myself to >>>>>>>>>>>> runner/core >>>>>>>>>>>> > because I wanted to take a look at the PRs related to >>>>>>>>>>>> > runner/core/metrics but I'm getting assigned to all >>>>>>>>>>>> runner-core PRs. Can >>>>>>>>>>>> > we specify paths like >>>>>>>>>>>> > >>>>>>>>>>>> runners/core-java/src/main/java/org/apache/beam/runners/core/metrics >>>>>>>>>>>> ? >>>>>>>>>>>> > I know it is a bit too precise so a bit risky, but in that >>>>>>>>>>>> particular >>>>>>>>>>>> > case, I doubt that the path will change. >>>>>>>>>>>> > >>>>>>>>>>>> > Etienne >>>>>>>>>>>> > >>>>>>>>>>>> > Le jeudi 12 juillet 2018 à 16:49 -0700, Eugene Kirpichov a >>>>>>>>>>>> écrit : >>>>>>>>>>>> >> Hi Udi, >>>>>>>>>>>> >> >>>>>>>>>>>> >> I see that the PR was merged - thanks! However it seems to >>>>>>>>>>>> have some >>>>>>>>>>>> >> unintended effects. >>>>>>>>>>>> >> >>>>>>>>>>>> >> On my PR https://github.com/apache/beam/pull/5940 , I >>>>>>>>>>>> assigned a >>>>>>>>>>>> >> reviewer manually, but the moment I pushed a new commit, it >>>>>>>>>>>> >> auto-assigned a lot of other people to it, and I had to >>>>>>>>>>>> remove them. >>>>>>>>>>>> >> This seems like a big inconvenience to me, is there a way to >>>>>>>>>>>> disable this? >>>>>>>>>>>> >> >>>>>>>>>>>> >> Thanks. >>>>>>>>>>>> >> >>>>>>>>>>>> >> On Thu, Jul 12, 2018 at 2:53 PM Udi Meiri < eh...@google.com >>>>>>>>>>>> >> <mailto: eh...@google.com>> wrote: >>>>>>>>>>>> >>> :/ That makes it a little less useful. >>>>>>>>>>>> >>> >>>>>>>>>>>> >>> On Thu, Jul 12, 2018 at 11:14 AM Tim Robertson >>>>>>>>>>>> >>> < timrobertson...@gmail.com <mailto: >>>>>>>>>>>> timrobertson...@gmail.com>> wrote: >>>>>>>>>>>> >>>> Hi Udi >>>>>>>>>>>> >>>> >>>>>>>>>>>> >>>> I asked the GH helpdesk and they confirmed that only >>>>>>>>>>>> people with >>>>>>>>>>>> >>>> write access will actually be automatically chosen. >>>>>>>>>>>> >>>> >>>>>>>>>>>> >>>> It don't expect it should stop us using it, but we should >>>>>>>>>>>> be aware >>>>>>>>>>>> >>>> that there are non-committers also willing to review. >>>>>>>>>>>> >>>> >>>>>>>>>>>> >>>> Thanks, >>>>>>>>>>>> >>>> Tim >>>>>>>>>>>> >>>> >>>>>>>>>>>> >>>> On Thu, Jul 12, 2018 at 7:24 PM, Mikhail Gryzykhin >>>>>>>>>>>> >>>> < mig...@google.com <mailto: mig...@google.com>> wrote: >>>>>>>>>>>> >>>>> Idea looks good in general. >>>>>>>>>>>> >>>>> >>>>>>>>>>>> >>>>> Did you look into ways to keep this file up-to-date? For >>>>>>>>>>>> example we >>>>>>>>>>>> >>>>> can run monthly job to see if owner was active during >>>>>>>>>>>> this period. >>>>>>>>>>>> >>>>> >>>>>>>>>>>> >>>>> --Mikhail >>>>>>>>>>>> >>>>> >>>>>>>>>>>> >>>>> Have feedback < http://go/migryz-feedback>? >>>>>>>>>>>> >>>>> >>>>>>>>>>>> >>>>> >>>>>>>>>>>> >>>>> On Thu, Jul 12, 2018 at 9:56 AM Udi Meiri < >>>>>>>>>>>> eh...@google.com >>>>>>>>>>>> >>>>> <mailto: eh...@google.com>> wrote: >>>>>>>>>>>> >>>>>> Thanks all! >>>>>>>>>>>> >>>>>> I'll try to get the file merged today and see how it >>>>>>>>>>>> works out. >>>>>>>>>>>> >>>>>> Please surface any issues, such as with auto-assignment, >>>>>>>>>>>> here or >>>>>>>>>>>> >>>>>> in JIRA. >>>>>>>>>>>> >>>>>> >>>>>>>>>>>> >>>>>> On Thu, Jul 12, 2018 at 2:12 AM Etienne Chauchot >>>>>>>>>>>> >>>>>> < echauc...@apache.org <mailto: echauc...@apache.org>> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>> Hi, >>>>>>>>>>>> >>>>>>> >>>>>>>>>>>> >>>>>>> I added myself as a reviewer for some modules. >>>>>>>>>>>> >>>>>>> >>>>>>>>>>>> >>>>>>> Etienne >>>>>>>>>>>> >>>>>>> >>>>>>>>>>>> >>>>>>> Le lundi 09 juillet 2018 à 17:06 -0700, Udi Meiri a >>>>>>>>>>>> écrit : >>>>>>>>>>>> >>>>>>>> Hi everyone, >>>>>>>>>>>> >>>>>>>> >>>>>>>>>>>> >>>>>>>> I'm proposing to add auto-reviewer-assignment using >>>>>>>>>>>> Github's >>>>>>>>>>>> >>>>>>>> CODEOWNERS mechanism. >>>>>>>>>>>> >>>>>>>> Initial version is >>>>>>>>>>>> >>>>>>>> here: _ >>>>>>>>>>>> https://github.com/apache/beam/pull/5909/files_ >>>>>>>>>>>> >>>>>>>> >>>>>>>>>>>> >>>>>>>> I need help from the community in determining owners >>>>>>>>>>>> for each >>>>>>>>>>>> >>>>>>>> component. >>>>>>>>>>>> >>>>>>>> Feel free to directly edit the PR (if you have >>>>>>>>>>>> permission) or >>>>>>>>>>>> >>>>>>>> add a comment. >>>>>>>>>>>> >>>>>>>> >>>>>>>>>>>> >>>>>>>> >>>>>>>>>>>> >>>>>>>> Background >>>>>>>>>>>> >>>>>>>> The idea is to: >>>>>>>>>>>> >>>>>>>> 1. Document good review candidates for each component. >>>>>>>>>>>> >>>>>>>> 2. Help choose reviewers using the auto-assignment >>>>>>>>>>>> mechanism. >>>>>>>>>>>> >>>>>>>> The suggestion is in no way binding. >>>>>>>>>>>> >>>>>>>> >>>>>>>>>>>> >>>>>>>> >>>>>>>>>>>> >>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Jean-Baptiste Onofré >>>>>>>>>>>> jbono...@apache.org >>>>>>>>>>>> http://blog.nanthrax.net >>>>>>>>>>>> Talend - http://www.talend.com >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Twitter: https://twitter.com/holdenkarau >>>>>>>> >>>>>>>
