I personally like blame-based suggestions. The downside is that you effectively become a owner of anything you touch. Most of the time blame based suggestions will return multiple candidates. Could we use the CODEOWNERS file to filter down the suggestions?
Andrew 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 >>> >>
