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é <[email protected]> wrote: > Agree to test it for a week. > > Regards > JB > Le 16 juil. 2018, à 18:59, Holden Karau <[email protected]> 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 < [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 >>>>>> >>>>>
