GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||
GeezerVison is showing github.com — tools: automate icu update by marco-ippolito · Pull Request #47727 · nodejs/node Conversation
Member
Refs: nodejs/security-wg#828
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Collaborator
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
marco-ippolito
force-pushed
the
feat/automate-icu-update
branch
from
7261c4a to
64c627c
Compare
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
23 tasks done
marco-ippolito
force-pushed
the
feat/automate-icu-update
branch
from
64c627c to
1eaaa62
Compare
richardlau
reviewed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should be validating this checksum matches the appropriate one from (We've had mismatches before.)
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and even check the GPG key against KEYS
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we check the generated checksum and the one deposited match? if they dont match we skip?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be my recommendation.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsUh oh!
There was an error while loading. Please reload this page.
|
|
cc FYI @srl295 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM with issues noted
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Great work, just needs some tweaking
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
debe898 to
44862bf
Compare
except that the in-repo one still has an |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think what you should do is extract the tz version from the ICU you're updating to, and put that in tz-version.txt.
Maybe
$ node -p process.versions.tz | tee test/fixtures/tz-version.txt
And then in reviewing the PR make sure it's not a regression.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Two requests (to recap):
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Also note that tz-version isn't the only thing that will break, there may be other test failures. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@srl295 should be good now |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Commit Queue failed- Loading data for nodejs/node/pull/47727 ✔ Done loading data for nodejs/node/pull/47727 ----------------------------------- PR info ------------------------------------ Title tools: automate icu update (#47727) Author Marco Ippolito (@marco-ippolito) Branch marco-ippolito:feat/automate-icu-update -> nodejs:main Labels meta, tools Commits 9 - tools: automate icu-small update - fix: replace - with . - fix: md5 from source - fix: comparing checksum - fix: mention the script in the maintaining guide - fix: lint - fix: update timezone.txt - fix: remove tz update - fix: use version as whole Committers 1 - Marco Ippolito PR-URL: https://github.com/nodejs/node/pull/47727 Refs: https://github.com/nodejs/security-wg/issues/828 Reviewed-By: Steven R Loomis Reviewed-By: Rafael Gonzaga ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/47727 Refs: https://github.com/nodejs/security-wg/issues/828 Reviewed-By: Steven R Loomis Reviewed-By: Rafael Gonzaga -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 26 Apr 2023 14:31:28 GMT ✔ Approvals: 2 ✔ - Steven R Loomis (@srl295): https://github.com/nodejs/node/pull/47727#pullrequestreview-1418824208 ✔ - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/47727#pullrequestreview-1417161365 ✔ Last GitHub CI successful ✘ No Jenkins CI runs detected -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/4927566555 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Commit Queue failed- Loading data for nodejs/node/pull/47727 ✔ Done loading data for nodejs/node/pull/47727 ----------------------------------- PR info ------------------------------------ Title tools: automate icu update (#47727) Author Marco Ippolito (@marco-ippolito) Branch marco-ippolito:feat/automate-icu-update -> nodejs:main Labels meta, tools Commits 9 - tools: automate icu-small update - fix: replace - with . - fix: md5 from source - fix: comparing checksum - fix: mention the script in the maintaining guide - fix: lint - fix: update timezone.txt - fix: remove tz update - fix: use version as whole Committers 1 - Marco Ippolito PR-URL: https://github.com/nodejs/node/pull/47727 Refs: https://github.com/nodejs/security-wg/issues/828 Reviewed-By: Steven R Loomis Reviewed-By: Rafael Gonzaga ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/47727 Refs: https://github.com/nodejs/security-wg/issues/828 Reviewed-By: Steven R Loomis Reviewed-By: Rafael Gonzaga -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 26 Apr 2023 14:31:28 GMT ✔ Approvals: 2 ✔ - Steven R Loomis (@srl295): https://github.com/nodejs/node/pull/47727#pullrequestreview-1418824208 ✔ - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/47727#pullrequestreview-1417161365 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-05-09T17:03:40Z: https://ci.nodejs.org/job/node-test-pull-request/51718/ - Querying data for job/node-test-pull-request/51718/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 47727 From https://github.com/nodejs/node * branch refs/pull/47727/merge -> FETCH_HEAD ✔ Fetched commits as 33231b0e8908..15ff4c572aa6 -------------------------------------------------------------------------------- Auto-merging .github/workflows/tools.yml [main 4f311994aa] tools: automate icu-small update Author: Marco Ippolito Date: Wed Apr 26 12:06:22 2023 +0200 3 files changed, 80 insertions(+) create mode 100755 tools/dep_updaters/update-icu.sh mode change 100644 => 100755 tools/icu/shrink-icu-src.py [main da76c2a76b] fix: replace - with . Author: Marco Ippolito Date: Wed Apr 26 18:03:27 2023 +0200 1 file changed, 4 insertions(+), 6 deletions(-) [main bdff48f4f9] fix: md5 from source Author: Marco Ippolito Date: Thu Apr 27 10:02:04 2023 +0200 1 file changed, 9 insertions(+), 4 deletions(-) [main 6c92d489d5] fix: comparing checksum Author: Marco Ippolito Date: Thu Apr 27 10:49:07 2023 +0200 1 file changed, 9 insertions(+) [main 3301ee5878] fix: mention the script in the maintaining guide Author: Marco Ippolito Date: Thu Apr 27 11:04:49 2023 +0200 1 file changed, 3 insertions(+) [main 361a6a53cc] fix: lint Author: Marco Ippolito Date: Thu Apr 27 12:05:29 2023 +0200 1 file changed, 1 insertion(+), 1 deletion(-) [main 8dc41afd62] fix: update timezone.txt Author: Marco Ippolito Date: Fri Apr 28 15:39:45 2023 +0200 1 file changed, 10 insertions(+) [main 03215a2cd9] fix: remove tz update Author: Marco Ippolito Date: Tue May 2 08:50:23 2023 +0200 1 file changed, 10 deletions(-) [main ca71746872] fix: use version as whole Author: Marco Ippolito Date: Tue May 2 09:02:24 2023 +0200 1 file changed, 5 insertions(+), 5 deletions(-) ✔ Patches applied There are 9 commits in the PR. Attempting autorebase. Rebasing (2/18)https://github.com/nodejs/node/actions/runs/4929182555 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Landed in 1b17793 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
PR-URL: #47727 Refs: nodejs/security-wg#828 Reviewed-By: Steven R Loomis <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: #47727 Refs: nodejs/security-wg#828 Reviewed-By: Steven R Loomis <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#47727 Refs: nodejs/security-wg#828 Reviewed-By: Steven R Loomis <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.