-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
zlib: remove mentions of unexposed Z_TREES constant #58371
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
zlib: remove mentions of unexposed Z_TREES constant #58371
Conversation
`Z_TREES` is never exposed on `DefineZlibConstants()`, hence it shall be removed. This fix is applicable to v18+. Refs: https://github.com/nodejs/node/blob/main/src/node_zlib.cc
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58371 +/- ##
==========================================
- Coverage 90.24% 90.23% -0.01%
==========================================
Files 633 633
Lines 186860 186860
Branches 36675 36684 +9
==========================================
- Hits 168623 168615 -8
- Misses 11035 11047 +12
+ Partials 7202 7198 -4
🚀 New features to boost your workflow:
|
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
Not sure why test fails. The report in https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel8-s390x/49510/testReport/junit/(root)/parallel/test_http2_debug/ does not tell me much. Would reviewers want to take a look on this? |
It is a known flake. Refs: #58353 |
Commit Queue failed- Loading data for nodejs/node/pull/58371 ✔ Done loading data for nodejs/node/pull/58371 ----------------------------------- PR info ------------------------------------ Title zlib: remove mentions of unexposed Z_TREES constant (#58371) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch hkleungai:zlib-remove-mentions-of-constants.Z_TREES -> nodejs:main Labels zlib, needs-ci Commits 1 - zlib: remove mentions of unexposed Z_TREES constant Committers 1 - Jimmy Leung <43258070+hkleungai@users.noreply.github.com> PR-URL: https://github.com/nodejs/node/pull/58371 Refs: https://github.com/nodejs/node/blob/main/src/node_zlib.cc Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/58371 Refs: https://github.com/nodejs/node/blob/main/src/node_zlib.cc Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 17 May 2025 18:47:10 GMT ✔ Approvals: 4 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/58371#pullrequestreview-2848438287 ✔ - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/58371#pullrequestreview-2848549721 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58371#pullrequestreview-2848916852 ✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/58371#pullrequestreview-2851541959 ✘ Last GitHub CI failed ℹ Last Full PR CI on 2025-05-18T16:58:52Z: https://ci.nodejs.org/job/node-test-pull-request/66899/ - Querying data for job/node-test-pull-request/66899/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/15243969971 |
Landed in c969649 |
Z_TREES
is never exposed onDefineZlibConstants()
, hence it shall beremoved.
This fix is applicable to v18+.
Refs: https://github.com/nodejs/node/blob/main/src/node_zlib.cc