It looks interesting but I would like to see the complete video and explanation about prow. Especially what we concretely need.
Regards JB Le 24 juil. 2018 à 04:17, à 04:17, Udi Meiri <eh...@google.com> a écrit: >I was recently told about Prow ><https://github.com/kubernetes/test-infra/tree/master/prow>, which >automates testing and merging for Kubernetes and other projects. >It also automates assigning reviewers and suggesting approvers. Example ><https://github.com/kubernetes/community/pull/2381> PR, video >explanation ><https://youtu.be/BsIC7gPkH5M?t=19m41s> >I propose trying out Prow, since is it's a maintained and it uses >OWNERS >files to explicitly define both who should be reviewing and who should >approve a PR. > >I'm not suggesting we use it to replace Jenkins or do our merges for >us. > > >On Tue, Jul 17, 2018 at 11:04 AM Udi Meiri <eh...@google.com> wrote: > >> +1 to generating the file. >> I'll go ahead and file a PR to remove CODEOWNERS >> >> On Tue, Jul 17, 2018 at 9:28 AM Holden Karau <hol...@pigscanfly.ca> >wrote: >> >>> So it doesn’t support doing that right now, although if we find it’s >a >>> problem we can specify an exclude file with folks who haven’t >contributed >>> in the past year. Would people want me to generate that first? >>> >>> On Tue, Jul 17, 2018 at 10:22 AM Ismaël Mejía <ieme...@gmail.com> >wrote: >>> >>>> Is there a way to put inactive people as not reviewers for the >blame >>>> case? I think it can be useful considering that a good amount of >our >>>> committers are not active at the moment and auto-assigning reviews >to >>>> them seem like a waste of energy/time. >>>> On Tue, Jul 17, 2018 at 1:58 AM Eugene Kirpichov ><kirpic...@google.com> >>>> wrote: >>>> > >>>> > We did not, but I think we should. So far, in 100% of the PRs >I've >>>> authored, the default functionality of CODEOWNERS did the wrong >thing and I >>>> had to fix something up manually. >>>> > >>>> > On Mon, Jul 16, 2018 at 3:42 PM Andrew Pilloud ><apill...@google.com> >>>> wrote: >>>> >> >>>> >> This sounds like a good plan. Did we want to rename the >CODEOWNERS >>>> file to disable github's mass adding of reviewers while we figure >this out? >>>> >> >>>> >> Andrew >>>> >> >>>> >> On Mon, Jul 16, 2018 at 10:20 AM Jean-Baptiste Onofré < >>>> j...@nanthrax.net> wrote: >>>> >>> >>>> >>> +1 >>>> >>> >>>> >>> Le 16 juil. 2018, à 19:17, Holden Karau ><holden.ka...@gmail.com> a >>>> écrit: >>>> >>>> >>>> >>>> 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é < >>>> j...@nanthrax.net> wrote: >>>> >>>>> >>>> >>>>> Agree to test it for a week. >>>> >>>>> >>>> >>>>> Regards >>>> >>>>> JB >>>> >>>>> Le 16 juil. 2018, à 18:59, Holden Karau < >holden.ka...@gmail.com> >>>> 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 < >>>> rfern...@google.com> wrote: >>>> >>>>>>> >>>> >>>>>>> +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 >>>> >>> -- >>> Twitter: https://twitter.com/holdenkarau >>> >>
