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

tools: automate zlib update by marco-ippolito · Pull Request #47417 · nodejs/node · GitHub

/ node Public

tools: automate zlib update - #47417

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:feat/automate-zlib-update
Apr 19, 2023
Merged

tools: automate zlib update#47417
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:feat/automate-zlib-update

Conversation

marco-ippolito commented Apr 5, 2023
edited
Loading

Copy link
Copy Markdown
Member

Refs: nodejs/security-wg#828
@nodejs/security-wg

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions

marco-ippolito requested a review from lpinca April 5, 2023 10:00
nodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Apr 5, 2023
marco-ippolito force-pushed the feat/automate-zlib-update branch 2 times, most recently from 6f313cd to c9f1101 Compare April 5, 2023 12:08
Comment thread tools/dep_updaters/update-zlib.sh Outdated

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

This works but it might remain the same for a very long time. It is not very useful. We refer to the tip of the remote main branch to check if an update is needed. However, afaik, we store the current commit SHA only in the commit message.

Copy link
Copy Markdown
Member 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

I havent found any other solution to check what's the latest version, I think this might be a reasonable compromise and its not very expensive

lpinca Apr 5, 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

That's fine but it probably won't find a new version for years. See https://github.com/madler/zlib/tags.

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

@lpinca are you saying that we typically update to something that is not a new release? If so then an option in the script/action to say generate PR based on commit X instead of a new version might make sense.

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

are you saying that we typically update to something that is not a new release?

Yes, exactly.

Copy link
Copy Markdown
Member 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

I think we should probably change it in the future to something like @tniessen did in this PR #47482

Comment thread tools/dep_updaters/update-zlib.sh Outdated

ShogunPanda left a comment

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

LGTM!

marco-ippolito added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 7, 2023
marco-ippolito force-pushed the feat/automate-zlib-update branch 2 times, most recently from b02200c to ef67324 Compare April 11, 2023 14:23
marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 12, 2023
nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 12, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/47417
✔  Done loading data for nodejs/node/pull/47417
----------------------------------- PR info ------------------------------------
Title      tools: automate zlib update (#47417)
Author     Marco Ippolito  (@marco-ippolito)
Branch     marco-ippolito:feat/automate-zlib-update -> nodejs:main
Labels     meta, tools, author ready
Commits    1
 - tools: automate zlib update
Committers 1
 - Marco Ippolito 
PR-URL: https://github.com/nodejs/node/pull/47417
Refs: https://github.com/nodejs/security-wg/issues/828
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/47417
Refs: https://github.com/nodejs/security-wg/issues/828
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - tools: automate zlib update
   ℹ  This PR was created on Wed, 05 Apr 2023 09:59:59 GMT
   ✔  Approvals: 2
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/47417#pullrequestreview-1376059006
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/47417#pullrequestreview-1376413213
   ⚠  This PR has conflicts that must be resolved
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/4678748046

marco-ippolito force-pushed the feat/automate-zlib-update branch from ef67324 to 7194af1 Compare April 12, 2023 13:28

RafaelGSS 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. Rebase to resolve the conflicts

marco-ippolito force-pushed the feat/automate-zlib-update branch from 7194af1 to 4c1a1a5 Compare April 17, 2023 07:23
marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 17, 2023
nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 17, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/47417
✔  Done loading data for nodejs/node/pull/47417
----------------------------------- PR info ------------------------------------
Title      tools: automate zlib update (#47417)
Author     Marco Ippolito  (@marco-ippolito)
Branch     marco-ippolito:feat/automate-zlib-update -> nodejs:main
Labels     meta, tools, author ready
Commits    1
 - tools: automate zlib update
Committers 1
 - Marco Ippolito 
PR-URL: https://github.com/nodejs/node/pull/47417
Refs: https://github.com/nodejs/security-wg/issues/828
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
Reviewed-By: Rafael Gonzaga 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/47417
Refs: https://github.com/nodejs/security-wg/issues/828
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
Reviewed-By: Rafael Gonzaga 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - tools: automate zlib update
   ℹ  This PR was created on Wed, 05 Apr 2023 09:59:59 GMT
   ✔  Approvals: 3
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/47417#pullrequestreview-1376059006
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/47417#pullrequestreview-1376413213
   ✔  - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/47417#pullrequestreview-1386302187
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/4723740689

marco-ippolito removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Apr 18, 2023
marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 18, 2023
nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 18, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/47417
✔  Done loading data for nodejs/node/pull/47417
----------------------------------- PR info ------------------------------------
Title      tools: automate zlib update (#47417)
Author     Marco Ippolito  (@marco-ippolito)
Branch     marco-ippolito:feat/automate-zlib-update -> nodejs:main
Labels     meta, tools, author ready
Commits    1
 - tools: automate zlib update
Committers 1
 - Marco Ippolito 
PR-URL: https://github.com/nodejs/node/pull/47417
Refs: https://github.com/nodejs/security-wg/issues/828
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
Reviewed-By: Rafael Gonzaga 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/47417
Refs: https://github.com/nodejs/security-wg/issues/828
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
Reviewed-By: Rafael Gonzaga 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - tools: automate zlib update
   ℹ  This PR was created on Wed, 05 Apr 2023 09:59:59 GMT
   ✔  Approvals: 3
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/47417#pullrequestreview-1376059006
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/47417#pullrequestreview-1376413213
   ✔  - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/47417#pullrequestreview-1386302187
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/4730739601

mhdawson 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 with a question.

marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 19, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 19, 2023
nodejs-github-bot merged commit 8f879de into nodejs:main Apr 19, 2023

Copy link
Copy Markdown
Collaborator

Landed in 8f879de

targos pushed a commit that referenced this pull request May 2, 2023
PR-URL: #47417
Refs: nodejs/security-wg#828
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
targos mentioned this pull request May 2, 2023
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
PR-URL: #47417
Refs: nodejs/security-wg#828
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
PR-URL: nodejs#47417
Refs: nodejs/security-wg#828
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.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

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL