-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
chore: add gyp-next updater #3105
base: main
Are you sure you want to change the base?
Conversation
954a5c8
to
ecf8f20
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.
Nice!
subprocess.check_output(["git", "commit", "-m", "feat(gyp): update gyp to " + args.tag]) | ||
if not args.no_commit: | ||
subprocess.check_output(["git", "add", "gyp"], cwd=CHECKOUT_PATH) | ||
subprocess.check_output(["git", "commit", "-m", "feat(gyp): update gyp to " + args.tag]) |
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.
subprocess.check_output(["git", "commit", "-m", "feat(gyp): update gyp to " + args.tag]) | |
subprocess.check_output([ | |
"git", "commit", "-m", "feat(gyp): update gyp to " + args.tag | |
]) |
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.
This PR is a great idea and will help a lot so I'm +1 on landing it.
The only change I'm requesting is that to use a more specific GitHub token that is only used for updating gyp-next
.
|
||
jobs: | ||
update-gyp-next: | ||
# if: github.repository == 'nodejs/node' || github.event_name == 'workflow_dispatch' |
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.
# if: github.repository == 'nodejs/node' || github.event_name == 'workflow_dispatch' |
@lukekarrys the new token permissions are tracked at nodejs/admin#935. It has the same permission needed for release-please and gyp-next updater as:
The reason to use a token for release-please is that the token GitHub actions created by default will not trigger GitHub actions:
So we need a token of @nodejs-github-bot, as requested at nodejs/admin#935. |
@legendecas Good call, I forgot about that restriction. I withdraw my previous suggestions and 👍 |
@legendecas What do you think about the suggestions in the unresolved comment threads? |
yes, I will update the PR according to the suggestions above once the token is setup. Thanks for catching them! |
Add automated updater for gyp-next.
Example: legendecas#1