Conversation
0e5a375 to
c3edc06
Compare
0649495 to
837073f
Compare
5c4f48f to
e7d67d0
Compare
VerteDinde
left a comment
There was a problem hiding this comment.
Overall it looks great! Left a few comments on what I thought could be potential breaking or unexpected changes, but am not going to block on them
| '-fno-rtti', | ||
| '-fno-exceptions', | ||
| '-fno-strict-aliasing', | ||
| - '-std=gnu++17', | ||
| + '-std=gnu++20', |
There was a problem hiding this comment.
Is changing this potentially going to break any native node modules for devs if they don't update their flags to be something similar? Just trying to think if we need to call out anything in the breaking changes doc.
There was a problem hiding this comment.
It shouldn't - this fixes a file split change but won't change anything more than was already changed in #43555!
| for await (const args of on(emitter, eventName)) { | ||
| if (untilFn(...args)) { return args; } | ||
| } | ||
| return []; |
There was a problem hiding this comment.
Does this change a return type here? 👀
| @@ -174,7 +174,6 @@ | ||
| 'src/timers.cc', | ||
| 'src/timer_wrap.cc', | ||
| 'src/tracing/agent.cc', |
There was a problem hiding this comment.
| #include <openssl/evp.h> | ||
| #include <openssl/hmac.h> | ||
| #include <openssl/pkcs12.h> | ||
| +#include <openssl/rand.h> |
| esm_drop_support_for_import_assertions.patch | ||
| fix_-wextra-semi_errors_in_nghttp2_helper_h.patch | ||
| fix_remove_harmony-import-assertions_from_node_cc.patch | ||
| win_almost_fix_race_detecting_esrch_in_uv_kill.patch |
There was a problem hiding this comment.
great work cleaning this up!
| @@ -1,5 +1,4 @@ | |||
| #include <node_api.h> | |||
| #undef NAPI_VERSION | |||
There was a problem hiding this comment.
This used to be necessary due to a conflict in #include <node_buffer.h> and #include <node_api.h> born of duplicate declarations, and isn't anymore as the duplicates got refactored out.
| }); | ||
|
|
||
| it('does not inherit parent env when custom env is provided', async () => { | ||
| // TODO(codebytere): figure out why this is failing in ASAN- builds on Linux. |
There was a problem hiding this comment.
This fails in ASAN but importantly does not cause a crash (the test just fails), so I'm disabling it for now to unblock the roll. There are no issues in non-ASAN builds.
| @@ -0,0 +1,70 @@ | |||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | |||
There was a problem hiding this comment.
This addresses nodejs/node#51766 - which we were seeing in CI on Windows.
|
Release Notes Persisted
|
This reverts commit c63d0d6.
Description of Change
Upgrade Node.js to v22.9.0.
Checklist
npm testpassesRelease Notes
Notes: Upgrades Node.js to v22.9.0.