Yea, great proposal. I expect we'll discover further refinements through experience more than deliberation, so I don't have any more comments on the doc.
Kenn On Mon, Feb 14, 2022 at 9:04 AM Kerry Donny-Clark <kerr...@google.com> wrote: > Thanks Danny, we can try this out and update as well. Everyone, please let > us know how this is working in practice once we roll it out. > Kerry > > On Mon, Feb 14, 2022 at 11:23 AM Danny McCormick < > dannymccorm...@google.com> wrote: > >> Thank you everyone who has chimed in here or on the doc - there's been a >> lot of good discussion and I think that will lead to a much better outcome! >> >> Since there's been general support for the idea and the flow of new >> comments tapered off a bit before the weekend, I'm going to go ahead and >> start to move forward with building out the automation (tracking JIRA here >> - https://issues.apache.org/jira/browse/BEAM-13925). Please feel free to >> leave any more thoughts in the doc and I promise I will respond/work to >> incorporate any thoughts that merit tweaking the design. >> >> Thanks, >> Danny >> >> On Fri, Feb 11, 2022 at 12:34 PM Robert Bradshaw <rober...@google.com> >> wrote: >> >>> This looks like a great plan! I remember being disappointed when >>> CODEOWNERs didn't meet our needs, but this looks like it resolves all >>> those issues. >>> >>> On Fri, Feb 11, 2022 at 9:02 AM Chamikara Jayalath <chamik...@google.com> >>> wrote: >>> > >>> > Thanks. I think this is shaping up to be a great proposal. >>> > >>> > - Cham >>> > >>> > On Fri, Feb 11, 2022 at 7:12 AM Jarek Potiuk <ja...@potiuk.com> wrote: >>> >> >>> >> Cool. Looking forward to see how it goes for Beam. We will also be at >>> the point soon that likely we will want to do something more sophisticated! >>> >> >>> >> On Fri, Feb 11, 2022 at 4:08 PM Danny McCormick < >>> dannymccorm...@google.com> wrote: >>> >>> >>> >>> Hey Jared, thanks for chiming in - I've been really appreciative of >>> the Airflow perspective (here and in the GitHub issues conversation), and >>> definitely hope we can keep learning from each other! We did consider >>> CODEOWNERs, but ultimately decided against it because it couldn't hit some >>> of our goals - specifically: >>> >>> >>> >>> 1. Providing multiple passes of assignment (once to a larger set of >>> reviewers, and then again to a second set of committers). >>> >>> >>> >>> 2. Balancing reviews - like you mentioned, there's not a great way >>> to do round robining, or even assign to a single person from a set of >>> people. Technically you can actually do this if every codeowner is part of >>> a team (https://twitter.com/github/status/1194673101117808653?lang=en), >>> but many Beam reviewers in our new model won't be a part of the Apache org. >>> (Maybe that feature would be of interest to Airflow though? It looks like >>> maybe all of your CODEOWNERS are part of the Apache org? I can't 100% tell). >>> >>> >>> >>> 3. Don't break the existing use case where a contributor wants a >>> review from a specific person. >>> >>> >>> >>> Thanks, >>> >>> Danny >>> >>> >>> >>> On Thu, Feb 10, 2022 at 7:52 AM Jarek Potiuk <ja...@potiuk.com> >>> wrote: >>> >>>> >>> >>>> Very interesting one - as an outsider I am interested to see how >>> this initiative will work out for the beam community. >>> >>>> >>> >>>> Just one comment - maybe you do not know but in GitHub there is a >>> "CODEOWNERS" feature (I notice you are not using it). Quote from >>> https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners >>> >>>> >>> >>>> | Code owners are automatically requested for review when someone >>> opens a pull request that modifies code that they own. Code owners are not >>> automatically requested to review draft pull requests. For more information >>> about draft pull requests, see "About pull requests." When you mark a draft >>> pull request as ready for review, code owners are automatically notified. >>> If you convert a pull request to a draft, people who are already subscribed >>> to notifications are not automatically unsubscribed. For more information, >>> see "Changing the stage of a pull request." >>> >>>> >>> >>>> This is an extremely poor version of what you try to do in Beam >>> (just assign everyone who is code owner as reviewer, no round-robin, no >>> reviewers role etc.), but maybe you want to try it quickly if you want to >>> test if any kind of "ownership" might help with at least initial vetting of >>> PRs. >>> >>>> This feature is enabled by literally committing one - >>> gitignore-like - file to repo, so it can be introduced extremely quickly. >>> >>>> >>> >>>> Airlfow's CODEOWNERS here as an example: >>> https://github.com/apache/airflow/blob/main/.github/CODEOWNERS >>> >>>> >>> >>>> J. >>> >>>> >>> >>>> On Thu, Feb 10, 2022 at 7:31 AM Ahmet Altay <al...@google.com> >>> wrote: >>> >>>>> >>> >>>>> Thank you Danny. I think this is a great problem to solve, and the >>> proposal looks great too :) I added comments as others but overall I like >>> it. >>> >>>>> >>> >>>>> On Wed, Feb 9, 2022 at 3:02 PM Brian Hulette <bhule...@google.com> >>> wrote: >>> >>>>>> >>> >>>>>> Thanks Danny! I left a few suggestions in the doc but I very much >>> like this idea overall. >>> >>>>>> >>> >>>>>> I especially like that "reviewers" is orthogonal to "committers", >>> giving new contributors a clear way to volunteer to help out with code >>> reviews. If we do this we should document it in the contribution guide [1]. >>> >>>>>> >>> >>>>>> [1] https://beam.apache.org/contribute/ >>> >>>>>> >>> >>>>>> On Wed, Feb 9, 2022 at 2:54 PM Kerry Donny-Clark < >>> kerr...@google.com> wrote: >>> >>>>>>> >>> >>>>>>> Danny, this looks like a great mechanism to ensure we review PRs >>> quickly and distribute the review work more evenly. >>> >>>>>>> Thanks for outlining a clear plan. I strongly support this. >>> >>>>>>> Kerry >>> >>>>>>> >>> >>>>>>> On Wed, Feb 9, 2022, 5:16 PM Danny McCormick < >>> dannymccorm...@google.com> wrote: >>> >>>>>>>> >>> >>>>>>>> Hey everyone, I put together a design doc for automating the >>> assignment of reviewers in Beam pull requests. I'd appreciate any thoughts >>> you have! >>> >>>>>>>> >>> >>>>>>>> Right now, we don't have a well defined automated system for >>> staying on top of pull request reviews - we rely on contributors being able >>> to find the correct OWNERS file and committers manually triaging/calling >>> attention to old pull requests. This doc proposes adding automation driven >>> by GitHub Actions to automatically round robin new PR reviews to a set of >>> contributors, thus balancing the load. It also proposes adding a new role >>> within the beam community of a reviewer who is responsible for an initial >>> code review on some PRs before they are routed to a committer for final >>> review. >>> >>>>>>>> >>> >>>>>>>> Please share any feedback or support here - >>> https://docs.google.com/document/d/1FhRPRD6VXkYlLAPhNfZB7y2Yese2FCWBzjx67d3TjBo/edit?usp=sharing >>> >>>>>>>> >>> >>>>>>>> Thanks, >>> >>>>>>>> Danny >>> >>
