-
Notifications
You must be signed in to change notification settings - Fork 30k
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
meta: automate description requests when notable change label is added #47078
meta: automate description requests when notable change label is added #47078
Conversation
Review requested:
|
bc3692a
to
d2123c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but unsure on how we can test it before landing.
that is what https://github.com/nodejs/node-auto-test is for |
I think new actions will not run on a PR as otherwise that would be an attack vector where anybody who submitted a PR could get us to run things. I think past best practice has been do do whatever testing you can in a personal repo and then land the PR and tweak through follow on PRs as necessary. |
I'm testing it on my fork and will report back |
Tested here: danielleadams#3 ✅ |
d2123c7
to
7723749
Compare
7723749
to
736c7fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what the intended action item is based on the message. Is it to post a new comment or to edit the one made by the bot?
- name: Add notable change description | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
run: gh pr comment ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --body "$NOTABLE_CHANGE_MESSAGE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using gh pr comment
like this is such a good idea. First time I've seen it, this is awesome.
@panva you can do either. See nodejs/Release#821 - the change is to have better descriptions of notable changes so that the release notes are more consistent across release lines (and developers get better overall release notes), so how it's communicated is up to the PR author. |
Let's come up with a better copy then, one that works for both collaborators who can edit the bot's comment and contributors who don't. I'll try to jot something down later today. |
The notable-changes label has been added by @${{ github.actor }}. | ||
Please include your text below if you'd like to include a more detailed summary in the changelog. | ||
``` | ||
[Insert here] | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notable-changes label has been added by @${{ github.actor }}. | |
Please include your text below if you'd like to include a more detailed summary in the changelog. | |
``` | |
[Insert here] | |
``` | |
The ${{ github.event.label.url }} label has been added by @${{ github.actor }}. | |
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. |
Something along these lines. Feel free to play around with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link will make the label appear like so
notable-change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@panva when I opened the PR, the event
provided a different url (danielleadams#4 (comment)), so going to replace with the hard coded link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
736c7fa
to
01d6451
Compare
01d6451
to
49f3a32
Compare
Landed in d612613 |
I don't think this was reflected... |
(testing, altho actions has degraded performance at this time so might take a while for the comment to appear, i'll keep tabs on this and follow up) |
The https://api.github.com/repos/nodejs/node/labels/notable-change label has been added by @panva. Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. |
PR-URL: #47078 Fixes: nodejs/Release#821 Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #47078 Fixes: nodejs/Release#821 Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #47078 Fixes: nodejs/Release#821 Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Adds a GH hook to leave a comment to ask for "Notable Change" descriptions.
Fixes: nodejs/Release#821