llvm / llvm-iwg

The LLVM Infrastructure Working Group
https://foundation.llvm.org/docs/infrastructure-wg/
Other
17 stars 14 forks source link

first ideas for a github notification tool #72

Closed ChristianKuehnel closed 2 years ago

ChristianKuehnel commented 2 years ago

First draft for "Gerald", a user-customizable notification tool for GitHub, similar to what Herald does for Phabricator.

ChristianKuehnel commented 2 years ago

Yes probably. I don't think it will contain the description of the PR/Issue :(

On Fri, Oct 1, 2021 at 10:23 PM Mehdi Amini @.***> wrote:

@.**** commented on this pull request.

In pull_request_migration/gerald_design.md https://github.com/llvm/llvm-iwg/pull/72#discussion_r720520089:

+Email-Mapping: +# For sending email notifications, we actually need to know the users email +# addresses. Github does not give out this. +- Elaine1234: @.** +`` + +Whenever a new PullRequest (PR) is opened Gerald first checks the PR +against this config file. If anAuto-Labelrule matches, Gerald assigns this +label to the PR. In the second phase Gerald checks theNotifications` rules +against new labels that were assigned to Issues or PRs. For all matches, Gerald +adds a comment: + +> GeraldBot commented on Aug 27th:* +> +> notifying @Elaine1234, @Gao1234 +

This is pretty good already, but I fear that the first email will look like:

@gerald https://github.com/gerald[bot] requested your review on: #1234

With nothing more, right?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/llvm/llvm-iwg/pull/72#pullrequestreview-769349196, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYJMCAZ32ZY734FG7OYDG3UEYKETANCNFSM5FEFIPIA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

joker-eph commented 2 years ago

Actually, since we have a similar tool in TensorFlow. I checked and when the bot is mentioning me on Pull-request GitHub sends me two email: first the one I didn't get on creation of the pull request with all the info, and then the mention from the bot. So it seems that GitHub is smart enough to adjust its notification here!

ChristianKuehnel commented 2 years ago

Based on the discussion on the llvm-dev mailing list: We can't store Personally Identifiable Information (PII) in our repos for privacy reasons. While we can keep public information like assigning labels to issues in the repo, we need to move all personal information somewhere private.

To store user data privately, we need some database and users need to authenticate. And that means we need to build a UI for managing the notifications. All of that make the project larger and requires more effort...

joker-eph commented 2 years ago

Are we considering a GitHub username as PII? Your Gerald proposal only needs to mention this and no email right? See what the Rust folks do: https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json#L14-L15

joker-eph commented 2 years ago

One more reference: CIRCT folks tried https://github.com/marketplace/actions/use-herald-action (but it was buggy so they stopped)

I was pointed out to https://github.com/marketplace/actions/auto-request-review (but without any particular experience with it)

ChristianKuehnel commented 2 years ago

Are we considering a GitHub username as PII? I'm not a privacy expert, but I would say this meets GitHub's definition

joker-eph commented 2 years ago

From the mailing list, this may also fit the bill! https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

(Note that, coming back to PII, it means that GitHub's own solution is to put usernames in a file in the repo...)

tstellar commented 2 years ago

I did a proof of concept notifier a while back. You can take a look at it here: https://github.com/llvm/temp-issue-tester

joker-eph commented 2 years ago

For more related work:

TensorFlow bot auto-assign pull-requests based on a config like this: https://github.com/tensorflow/tensorflow/blob/master/.github/bot_config.yml

This is a custom bot I believe, it also has a config to ping when a pull-request is inactive for a while (and later close it): https://github.com/tensorflow/tensorflow/blob/master/.github/stale.yml#L22-L38

joker-eph commented 2 years ago

Another one mentioned in the RFC thread: https://github.com/gagoar/use-herald-action

ChristianKuehnel commented 2 years ago

Another one mentioned in the RFC thread: https://github.com/gagoar/use-herald-action

They are also storing the user names in config files in the repo. I'm not sure if that is what we want.

ChristianKuehnel commented 2 years ago

closing as we already have another solution in place

joker-eph commented 2 years ago

Are you referring to the team management for GH Issues? I don't think it is a direct replacement to Herald for pull-request.