Skip to content

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Aug 11, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 11, 2018
@vsemozhetbyt
Copy link
Contributor

src/node.cc Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were the NODE_HAVE_I18N_SUPPORT checks about?

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 11, 2018

FWIW, there already were some doubts about the connection between Intl support and some CLI flags in #19878

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd rather be consistent with the existing entries in terms of style.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test still fails. Is NODE_PRESERVE_SYMLINKS still needed in this check?

@devsnek
Copy link
Member Author

devsnek commented Aug 16, 2018

@vsemozhetbyt
Copy link
Contributor

@devsnek devsnek closed this Aug 16, 2018
@devsnek devsnek merged commit be9f786 into nodejs:master Aug 16, 2018
@devsnek devsnek deleted the fix/cli-help branch August 16, 2018 16:13
targos pushed a commit that referenced this pull request Aug 19, 2018
PR-URL: #22271
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
targos pushed a commit that referenced this pull request Sep 3, 2018
PR-URL: #22271
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants