-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
build: add -Werror=undefined-inline to clang builds #23961
Conversation
fa54260
to
ad46b32
Compare
I guess we should block on #23954? |
Fails as expected on macOS (with clang)
Waiting for #23954 |
Why didn't it fail on FreeBSD? |
Probably because the |
#23954 landed so unblocked. |
@refack Did you forget to rebase? It's still failing with the same build error. |
ad46b32
to
9304541
Compare
pushed without force, and didn't look at results... new resume: https://ci.nodejs.org/job/node-test-pull-request/18260/ |
PR-URL: nodejs#23961 Refs: nodejs#23954 Refs: nodejs#23910 Refs: nodejs#23880 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
9304541
to
c515e5c
Compare
Two more hours, so lets CI again: https://ci.nodejs.org/job/node-test-pull-request/18262/ |
opted to land on 10.x but not 8.x |
Not only it breaks the build with GCC, but it gets pulled out from somewhere even after being patched out. |
Hopefully the start of a trend to move more cases to
-Werror
Refs: #23954
Refs: #23910
Refs: #23880
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes