FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

meta: automate description requests when notable change label is added by danielleadams · Pull Request #47078 · nodejs/node · GitHub

/ node Public

meta: automate description requests when notable change label is added - #47078

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
danielleadams:notable-change-bot
Mar 24, 2023
Merged

meta: automate description requests when notable change label is added#47078
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
danielleadams:notable-change-bot

Conversation

danielleadams commented Mar 14, 2023
edited
Loading

Copy link
Copy Markdown
Contributor

Adds a GH hook to leave a comment to ask for "Notable Change" descriptions.

Fixes: nodejs/Release#821

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions

danielleadams added the notable-change PRs with changes that should be highlighted in changelogs. label Mar 14, 2023
nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Mar 14, 2023
danielleadams removed the notable-change PRs with changes that should be highlighted in changelogs. label Mar 14, 2023
danielleadams marked this pull request as ready for review March 14, 2023 02:24
danielleadams added the notable-change PRs with changes that should be highlighted in changelogs. label Mar 14, 2023
danielleadams deleted the notable-change-bot branch March 15, 2023 11:42
danielleadams restored the notable-change-bot branch March 15, 2023 17:44
danielleadams reopened this Mar 15, 2023
danielleadams marked this pull request as draft March 15, 2023 17:45
danielleadams marked this pull request as ready for review March 15, 2023 17:46
danielleadams removed the notable-change PRs with changes that should be highlighted in changelogs. label Mar 15, 2023

BethGriggs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, but unsure on how we can test it before landing.

MoLow commented Mar 15, 2023

Copy link
Copy Markdown
Member

LGTM, but unsure on how we can test it before landing.

that is what https://github.com/nodejs/node-auto-test is for

danielleadams changed the title chore: automate description requests when notable change label is added meta: automate description requests when notable change label is added Mar 15, 2023

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

I'm testing it on my fork and will report back

danielleadams commented Mar 16, 2023
edited
Loading

Copy link
Copy Markdown
Contributor Author

Tested here: danielleadams#3

panva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Comment thread .github/workflows/comment-labeled.yml Outdated

bnb Mar 16, 2023
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

using gh pr comment like this is such a good idea. First time I've seen it, this is awesome.

Copy link
Copy Markdown
Contributor Author

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?

@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.

panva commented Mar 16, 2023

Copy link
Copy Markdown
Member

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?

@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.

Comment thread .github/workflows/comment-labeled.yml Outdated
Comment on lines 14 to 18

panva Mar 16, 2023
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The link will make the label appear like so notable-change PRs with changes that should be highlighted in changelogs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@panva when I opened the PR, the event provided a different url (danielleadams#4 (comment)), so going to replace with the hard coded link.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure.

Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 24, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 24, 2023
nodejs-github-bot merged commit d612613 into nodejs:main Mar 24, 2023

Copy link
Copy Markdown
Collaborator

Landed in d612613

panva commented Mar 29, 2023

Copy link
Copy Markdown
Member

#47078 (comment)

I don't think this was reflected...

panva added the notable-change PRs with changes that should be highlighted in changelogs. label Mar 29, 2023

panva commented Mar 29, 2023
edited
Loading

Copy link
Copy Markdown
Member

(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)

panva added notable-change PRs with changes that should be highlighted in changelogs. and removed notable-change PRs with changes that should be highlighted in changelogs. labels Mar 29, 2023

Copy link
Copy Markdown
Contributor

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.

panva removed the notable-change PRs with changes that should be highlighted in changelogs. label Mar 29, 2023

panva commented Mar 29, 2023

Copy link
Copy Markdown
Member

#47300

RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
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>
RafaelGSS mentioned this pull request Apr 6, 2023
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
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>
danielleadams added a commit that referenced this pull request Jul 6, 2023
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Issues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: notable-changes bot comment for summaries

9 participants


Back | FazBrowse Home | New Git URL