We did not, but I think we should. So far, in 100% of the PRs I've authored, the default functionality of CODEOWNERS did the wrong thing and I had to fix something up manually.
On Mon, Jul 16, 2018 at 3:42 PM Andrew Pilloud <apill...@google.com> wrote: > 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 >>>>>>>>> >>>>>>>>
