Skip to content

fix: avoid quoting cflag name and parameter with space separator#223

Merged
cclauss merged 4 commits into
nodejs:mainfrom
legendecas:compile_commands_macos
Mar 7, 2024
Merged

fix: avoid quoting cflag name and parameter with space separator#223
cclauss merged 4 commits into
nodejs:mainfrom
legendecas:compile_commands_macos

Conversation

@legendecas

@legendecas legendecas commented Mar 1, 2024

Copy link
Copy Markdown
Member

Fixes: nodejs/node-gyp#2858

Avoid quoting -arch arm64 in generated compile_commands.json as c++ "-arch arm64".

@legendecas
legendecas marked this pull request as ready for review March 1, 2024 07:21
@legendecas

Copy link
Copy Markdown
Member Author

@nodejs/gyp I'd appreciate it if this could have a review, thank you!

@cclauss cclauss 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.

Please consider adding tests that fail without these changes but pass with these changes so that we can avoid future regressions.

@cclauss cclauss 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.

Nice!

Perhaps create a DRAFT PR with only the test changes to prove that the tests fail on the unmodified code

@legendecas

Copy link
Copy Markdown
Member Author

Sure, #224

@cclauss
cclauss merged commit 2b9703d into nodejs:main Mar 7, 2024
@legendecas
legendecas deleted the compile_commands_macos branch March 7, 2024 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clangd complains for Unknown argument: '-arch arm64' [drv_unknown_argument]

2 participants