Hey all, So finally I had some time for some lower prio stuff since everyone is still on holiday. I've implemented this Github action check in a PR: https://github.com/apache/kafka/pull/15115 It uses the Github API to fetch the name and email of the user based on the username who submitted the review. If this is still something that the community wants then please let me know with a review :). I've tested this on my own fork here: https://github.com/viktorsomogyi/kafka/pull/4. I think you can also try this by raising a PR against the trunk of my fork (although I don't know if I need to add you as a collaborator but if so, then I'll happily do it for this test).
Thanks, Viktor On Wed, May 3, 2023 at 9:41 AM Viktor Somogyi-Vass < viktor.somo...@cloudera.com> wrote: > Yes, perhaps this can be used in the github action too, I think this is a > very useful tool. Sadly I couldn't get to the github action but hopefully I > will get there soon. > > On Fri, Apr 28, 2023 at 8:48 AM David Jacot <dja...@confluent.io.invalid> > wrote: > >> Thanks, David. This is a nice addition! >> >> Coming back to the original proposal of using github actions, it may be >> possible to run David's script automatically. For instance, we could >> trigger an action which pulls the folks who have approved the PR and feed >> the script when a comment with `reviewers` is posted. Then the action >> would >> post a comment with the "Reviewers: ....". This way, we could do >> everything from within the PR. >> >> Cheers, >> David >> >> On Thu, Apr 27, 2023 at 8:35 PM David Arthur >> <david.art...@confluent.io.invalid> wrote: >> >> > I just merged the "reviewers" script I wrote a while ago: >> > https://github.com/apache/kafka/pull/11096 >> > >> > It works by finding previous occurrences of "Reviewers: ...", so it only >> > works for people who have reviewed something before. I do suspect this >> is >> > largely the common case. >> > >> > E.g., searching for "Ismael" gives: >> > >> > Possible matches (in order of most recent): >> > [1] Ismael Juma ism...@juma.me.uk (1514) >> > [2] Ismael Juma ij...@apache.org (3) >> > [3] Ismael Juma mli...@juma.me.uk (4) >> > [4] Ismael Juma ism...@confluent.io (19) >> > [5] Ismael Juma git...@juma.me.uk (7) >> > >> > it shows them in order of most recently occurring along with the number >> of >> > occurrences. Now that it's merged, it should be easier for folks to try >> it >> > out. >> > >> > Cheers, >> > David >> > >> > On Thu, Apr 20, 2023 at 1:02 PM Justine Olshan >> > <jols...@confluent.io.invalid> >> > wrote: >> > >> > > I've tried the script, but it's not quite complete. >> > > I've had issues finding folks -- if they haven't reviewed in kafka, we >> > can >> > > not find an email for them. I also had some issues with finding folks >> who >> > > had reviewed before. >> > > >> > > Right now, my strategy is to use GitHub to search previous commits for >> > > folks' emails, but that isn't the most optimal solution -- especially >> if >> > > the reviewer has no public email. >> > > I do think it is useful to have in the commit though, so if anyone has >> > some >> > > ideas on how to improve, I'd be happy to hear. >> > > >> > > Justine >> > > >> > > On Wed, Apr 19, 2023 at 6:53 AM Ismael Juma <ism...@juma.me.uk> >> wrote: >> > > >> > > > It's a lot more convenient to have it in the commit than having to >> > follow >> > > > links, etc. >> > > > >> > > > David Arthur also wrote a script to help with this step, I believe. >> > > > >> > > > Ismael >> > > > >> > > > On Tue, Apr 18, 2023, 9:29 AM Divij Vaidya <divijvaidy...@gmail.com >> > >> > > > wrote: >> > > > >> > > > > Do we even need a manual attribution for a reviewer in the commit >> > > > message? >> > > > > GitHub automatically marks the folks as "reviewers" who have used >> the >> > > > > "review-changes" button on the top left corner and left feedback. >> > > GitHub >> > > > > also has searchability for such reviews done by a particular >> person >> > > using >> > > > > the following link: >> > > > > >> > > > > https://github.com/search?q=is%3Apr+reviewed-by%3A >> > > > > >> > > > >> > > >> > >> <add-reviewer>+repo%3Aapache%2Fkafka+repo%3Aapache%2Fkafka-site&type=issues >> > > > > >> > > > > (replace <add-reviewer> with the GitHub username) >> > > > > >> > > > > -- >> > > > > Divij Vaidya >> > > > > >> > > > > >> > > > > >> > > > > On Tue, Apr 18, 2023 at 4:09 PM Viktor Somogyi-Vass >> > > > > <viktor.somo...@cloudera.com.invalid> wrote: >> > > > > >> > > > > > I'm not that familiar with Actions either, it just seemed like a >> > tool >> > > > for >> > > > > > this purpose. :) >> > > > > > I Did some digging and what I have in mind is that on pull >> request >> > > > review >> > > > > > it can trigger a workflow: >> > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review >> > > > > > >> > > > > > We could in theory use Github CLI to edit the description of >> the PR >> > > > when >> > > > > > someone gives a review (or we could perhaps enable this to >> simply >> > > > comment >> > > > > > too): >> > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> https://docs.github.com/en/actions/using-workflows/using-github-cli-in-workflows >> > > > > > >> > > > > > So the action definition would look something like this below. >> Note >> > > > that >> > > > > > the "run" part is very basic, it's just here for the idea. We'll >> > > > probably >> > > > > > need a shell script instead of that line to format it better. >> But >> > the >> > > > > point >> > > > > > is that it edits the PR and adds the reviewer: >> > > > > > >> > > > > > name: Add revieweron: >> > > > > > issues: >> > > > > > types: >> > > > > > - pull_request_reviewjobs: >> > > > > > comment: >> > > > > > runs-on: ubuntu-latest >> > > > > > steps: - run: gh pr edit $PR_ID --title "$PR_TITLE" >> --body >> > > > > > "$PR_BODY\n\nReviewers: $SENDER" >> > > > > > env: >> > > > > > GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} >> > > > > > PR_ID: ${{ github.event.pull_request.id }} >> > > > > > PR_TITLE: ${{ github.event.pull_request.title }} >> > > > > > PR_BODY: ${{ github.event.pull_request.body }} >> > > > > > SENDER: ${{ github.event.sender }} >> > > > > > >> > > > > > I'll take a look if I can try this out one my fork and get back >> if >> > it >> > > > > leads >> > > > > > to anything. >> > > > > > >> > > > > > Viktor >> > > > > > >> > > > > > On Tue, Apr 18, 2023 at 10:12 AM Josep Prat >> > > > <josep.p...@aiven.io.invalid >> > > > > > >> > > > > > wrote: >> > > > > > >> > > > > > > Hi all, >> > > > > > > Unless I miss something, wouldn't this GitHub action either >> amend >> > > the >> > > > > > > commit (breaking signature if any) or directly do the commit >> > itself >> > > > > > > (meaning the action would be the one squashing and merging and >> > not >> > > > the >> > > > > > > maintainer anymore)? >> > > > > > > >> > > > > > > Let me know if I'm missing something or if there are some nice >> > > hidden >> > > > > > > tricks in GitHub that I didn't know :) >> > > > > > > >> > > > > > > Best, >> > > > > > > On Tue, Apr 18, 2023 at 9:48 AM Viktor Somogyi-Vass >> > > > > > > <viktor.somo...@cloudera.com.invalid> wrote: >> > > > > > > >> > > > > > > > Hi all, >> > > > > > > > >> > > > > > > > Unfortunately I forgot to add myself as a reviewer *again >> *on a >> > > PR >> > > > > when >> > > > > > > > merging. Shame on me. >> > > > > > > > However I was thinking about looking into Github actions >> > whether >> > > we >> > > > > can >> > > > > > > > automate this process or at least prevent PRs from merging >> that >> > > > don't >> > > > > > > have >> > > > > > > > "reviewers" in the description. >> > > > > > > > >> > > > > > > > Has anyone ever looked at it, is it worth chasing this or >> does >> > > > anyone >> > > > > > > know >> > > > > > > > anything that'd prevent us from using it? >> > > > > > > > >> > > > > > > > Viktor >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > -- >> > > > > > > [image: Aiven] <https://www.aiven.io> >> > > > > > > >> > > > > > > *Josep Prat* >> > > > > > > Open Source Engineering Director, *Aiven* >> > > > > > > josep.p...@aiven.io | +491715557497 >> > > > > > > aiven.io <https://www.aiven.io> | < >> > > > > > https://www.facebook.com/aivencloud >> > > > > > > > >> > > > > > > <https://www.linkedin.com/company/aiven/> < >> > > > > > > https://twitter.com/aiven_io> >> > > > > > > *Aiven Deutschland GmbH* >> > > > > > > Alexanderufer 3-7, 10117 Berlin >> > > > > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen >> > > > > > > Amtsgericht Charlottenburg, HRB 209739 B >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> > >> > -- >> > -David >> > >> >
