+1. This is great. On Sat, Jul 14, 2018 at 7:44 AM Udi Meiri <[email protected]> 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 <[email protected]> > 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 <[email protected]> >> 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 <[email protected]> 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 <[email protected]> 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é <[email protected]> >>>>> 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 <[email protected] >>>>>> >> <mailto:[email protected]>> wrote: >>>>>> >>> :/ That makes it a little less useful. >>>>>> >>> >>>>>> >>> On Thu, Jul 12, 2018 at 11:14 AM Tim Robertson >>>>>> >>> <[email protected] <mailto:[email protected]>> >>>>>> 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 >>>>>> >>>> <[email protected] <mailto:[email protected]>> 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 <[email protected] >>>>>> >>>>> <mailto:[email protected]>> 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 >>>>>> >>>>>> <[email protected] <mailto:[email protected]>> 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é >>>>>> [email protected] >>>>>> http://blog.nanthrax.net >>>>>> Talend - http://www.talend.com >>>>>> >>>>> >> >> >> -- >> Twitter: https://twitter.com/holdenkarau >> >
