-
Notifications
You must be signed in to change notification settings - Fork 113
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: use getUrlFromOP()
for fixes
links
#614
Conversation
When parsing `fixes` links, use the same code that is used for parsing the `refs` and `PR-URL` links. This avoids the manual URL construction that was inserting bad links into the metadata for the npm update pull requests.
Codecov Report
@@ Coverage Diff @@
## main #614 +/- ##
==========================================
- Coverage 84.10% 84.09% -0.01%
==========================================
Files 37 37
Lines 4051 4049 -2
==========================================
- Hits 3407 3405 -2
Misses 644 644
Continue to review full report at Codecov.
|
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
Anyone have any ideas about the coverage drop/failure? Are we able to land with this check failing? FWIW TIL that $ npx node-core-utils 42550
npx: installed 493 in 17.679s
✔ Done loading data for nodejs/node/pull/42550
----------------------------------- PR info ------------------------------------
Title deps: upgrade npm to 8.6.0 (#42550)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch npm-robot:npm-8.6.0 -> nodejs:master
Labels npm, fast-track, needs-ci, dont-land-on-v12.x, dont-land-on-v14.x
Commits 1
- deps: upgrade npm to 8.6.0
Committers 1
- npm team <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42550
Fixes: https://github.com/remove
Fixes: https://github.com/100%
Fixes: https://github.com/make
Fixes: https://github.com/move
Fixes: https://github.com/really
Fixes: https://github.com/consolidate
Fixes: https://github.com/consolidate
Fixes: https://github.com/consolidate
Fixes: https://github.com/consolidate
Fixes: https://github.com/bump
Fixes: https://github.com/return
Fixes: https://github.com/consolidate
Fixes: https://github.com/work
Fixes: https://github.com/only
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
--------------------------------------------------------------------------------
ℹ This PR was created on Thu, 31 Mar 2022 22:43:19 GMT
✔ Approvals: 4
✔ - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/42550#pullrequestreview-928307710
✔ - Mestery (@Mesteery): https://github.com/nodejs/node/pull/42550#pullrequestreview-928452353
✔ - Mohammed Keyvanzadeh (@VoltrexMaster): https://github.com/nodejs/node/pull/42550#pullrequestreview-928474922
✔ - Tierney Cyren (@bnb): https://github.com/nodejs/node/pull/42550#pullrequestreview-928474971
ℹ This PR is being fast-tracked
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2022-04-01T08:44:02Z: https://ci.nodejs.org/job/node-test-pull-request/43255/
✔ Build data downloaded
✔ Last Jenkins CI successful while with this PR: $ node bin/get-metadata.js 42550
✔ Done loading data for nodejs/node/pull/42550
----------------------------------- PR info ------------------------------------
Title deps: upgrade npm to 8.6.0 (#42550)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch npm-robot:npm-8.6.0 -> nodejs:master
Labels npm, fast-track, needs-ci, dont-land-on-v12.x, dont-land-on-v14.x
Commits 1
- deps: upgrade npm to 8.6.0
Committers 1
- npm team <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42550
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
--------------------------------------------------------------------------------
ℹ This PR was created on Thu, 31 Mar 2022 22:43:19 GMT
✔ Approvals: 4
✔ - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/42550#pullrequestreview-928307710
✔ - Mestery (@Mesteery): https://github.com/nodejs/node/pull/42550#pullrequestreview-928452353
✔ - Mohammed Keyvanzadeh (@VoltrexMaster): https://github.com/nodejs/node/pull/42550#pullrequestreview-928474922
✔ - Tierney Cyren (@bnb): https://github.com/nodejs/node/pull/42550#pullrequestreview-928474971
ℹ This PR is being fast-tracked
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2022-04-01T08:44:02Z: https://ci.nodejs.org/job/node-test-pull-request/43255/
✔ Build data downloaded
✔ Last Jenkins CI successful
|
Yes. I'll land. |
When parsing
fixes
links, use the same code that is used for parsingthe
refs
andPR-URL
links. This avoids the manual URL constructionthat was inserting bad links into the metadata for the npm update pull
requests.
Refs: nodejs/node#42382 (comment)
This is an alternative to #612 which also fixes the case in #612 (comment).