Skip to content
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

src: remove node_options-inl.h from header files #27538

Closed
wants to merge 1 commit into from

Conversation

sam-github
Copy link
Contributor

@sam-github sam-github commented May 2, 2019

Fix unnecessary inclusion of node_options-inl.h into env.h via
inspector_agent.h, causing almost all of src/ to recompile on any change
to the options parser. Its intended that *-inl.h header files are only
included into the src files that call the inline methods.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Fix unnecessary inclusion of node_options-inl.h into env.h via
inspector_agent.h, causing almost all of src/ to recompile on any change
to the options parser. Its intended that *-inl.h header files are only
included into the src files that call the inline methods.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels May 2, 2019
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 2, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/22891/ (:heavy_check_mark:)

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 3, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request May 5, 2019
Fix unnecessary inclusion of node_options-inl.h into env.h via
inspector_agent.h, causing almost all of src/ to recompile on any change
to the options parser. Its intended that *-inl.h header files are only
included into the src files that call the inline methods.

PR-URL: nodejs#27538
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott
Copy link
Member

Trott commented May 5, 2019

Landed in c21693b

@Trott Trott closed this May 5, 2019
targos pushed a commit that referenced this pull request May 5, 2019
Fix unnecessary inclusion of node_options-inl.h into env.h via
inspector_agent.h, causing almost all of src/ to recompile on any change
to the options parser. Its intended that *-inl.h header files are only
included into the src files that call the inline methods.

PR-URL: #27538
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@targos targos mentioned this pull request May 6, 2019
@sam-github sam-github deleted the remove-inl-from-env.h branch May 23, 2019 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants