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 <[email protected]> wrote: > +1 using blame -- nifty :) > > On Mon, Jul 16, 2018 at 2:31 AM Huygaa Batsaikhan <[email protected]> > wrote: > >> +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 >>>> >>>
