+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 >>> >>
smime.p7s
Description: S/MIME Cryptographic Signature
