-
Notifications
You must be signed in to change notification settings - Fork 399
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
fix: Set username/email to machine user by default #1627
fix: Set username/email to machine user by default #1627
Conversation
Signed-off-by: mrickard <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1627 +/- ##
==========================================
+ Coverage 96.52% 96.55% +0.03%
==========================================
Files 200 200
Lines 39039 39043 +4
Branches 24 24
==========================================
+ Hits 37681 37699 +18
+ Misses 1358 1344 -14
Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
a tiny comment about no longer being able to support running this locally
bin/create-docs-pr.js
Outdated
@@ -281,7 +284,7 @@ async function createPR(username, version, branch, dryRun, repoOwner) { | |||
const github = new Github(repoOwner, 'docs-website') | |||
const title = `Node.js Agent ${version} Release Notes` | |||
const prOptions = { | |||
head: `${username}:${branch}`, | |||
head: `${branch}`, |
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 won't work when we run locally but it seems like repoOwner and username were redundant in this case right?
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've updated this to run locally, though it's using repoOwner for the fork instead of username.
…e main repo. Otherwise, if run locally, it should be to a fork. Signed-off-by: mrickard <[email protected]>
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 cannot run this locally when i specify repo-path
. it tries to clone with a github token. it should not try to clone if repot-path is set or at least that was legacy behavior. I don't think we want to clone the repo every time
…eady exists locally. Signed-off-by: mrickard <[email protected]>
@bizob2828 I've updated so that the docs repo won't be cloned if it already exists locally on repo-path. Mind taking another look? |
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.
Verified local run still works. We can check if automate works I guess on next release
Proposed Release Notes
Changed post-release create-docs-pr job to use the machine user username/email, and removed the fork syntax from the docs PR.
Links
NR-96240
Details