Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Github] Add PR author name to subscription email #68440

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

boomanaiden154
Copy link
Contributor

Currently the email that gets sent out to people subscribing to a label that the bot tags on the PR doesn't include any authorship information which some people are interested in having. This patch adds an author field to the message with the relevant information.

Currently the email that gets sent out to people subscribing to a label
that the bot tags on the PR doesn't include any authorship information
which some people are interested in having. This patch adds an author
field to the message with the relevant information.
@boomanaiden154
Copy link
Contributor Author

These changes should be correct but I haven't tested them out. Not sure what the testing procedure is like. Try it out on my fork after adding the relevant token secrets?

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 6, 2023

Probably worthwhile for issues too?

@joker-eph
Copy link
Collaborator

Try it out on my fork after adding the relevant token secrets?

You can try it out on a this PR by running the script yourself locally with your token. We should see some result in a comment here directly.

@boomanaiden154
Copy link
Contributor Author

@llvm/pr-subscribers-llvm-exegesis

Author: Aiden Grossman (boomanaiden154)

Changes

Currently the email that gets sent out to people subscribing to a label that the bot tags on the PR doesn't include any authorship information which some people are interested in having. This patch adds an author field to the message with the relevant information.


Full diff: https://github.com/llvm/llvm-project/pull/68440.diff

1 Files Affected:

  • (modified) llvm/utils/git/github-automation.py (+2)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index b90c68a3618b9d7..cd549e75bf7d6cd 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -165,6 +165,8 @@ def run(self) -> bool:
 {self.COMMENT_TAG}
 {team_mention}
 
+Author: {self.pr._user.name} ({self.pr._user.login})
+
 <details>
 <summary>Changes</summary>
 

@boomanaiden154
Copy link
Contributor Author

Probably worthwhile for issues too?

Thanks for the suggestion! I've implemented it for issues as well. The example is available here.

You can try it out on a this PR by running the script yourself locally with your token. We should see some result in a comment here directly.

Thanks! That worked pretty much perfectly. Maybe worth documenting somewhere at some point (if it isn't already).

@boomanaiden154 boomanaiden154 merged commit f8148e4 into llvm:main Oct 9, 2023
2 of 3 checks passed
@mstorsjo
Copy link
Member

I just wanted to say thanks for implementing this! I've been meaning to look at doing exactly this for a couple of weeks now, but haven't gotten to it yet.

@Endilll Endilll added the infrastructure Bugs about LLVM infrastructure label Oct 24, 2023
@Endilll
Copy link
Contributor

Endilll commented Oct 24, 2023

It would be nice to extract mannequin names as well, because issues migrated from bugzilla are still significant. Example: #49522 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Bugs about LLVM infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants