Skip to content

src: order C++ error list alphabetically - #20707

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:cpp-errors-alphabetical
Closed

src: order C++ error list alphabetically#20707
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:cpp-errors-alphabetical

Conversation

@addaleax

@addaleaxaddaleax commented May 13, 2018

Copy link
Copy Markdown
Member
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-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 13, 2018
@addaleax

Copy link
Copy Markdown
MemberAuthor

Comment threadsrc/node_errors.h
@@ -31,7 +32,6 @@ namespace node {
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \
V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \
V(ERR_STRING_TOO_LONG, Error) \

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.

Doesn't the \ at the end of this line need to be removed now?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

We actually try to keep those, so that adding/removing lines has lower overhead, i.e. the previous style was actually the less common one.

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 14, 2018
@addaleax

Copy link
Copy Markdown
MemberAuthor

@apapirovski

Copy link
Copy Markdown
Contributor

Landed in 34471c7

apapirovski pushed a commit that referenced this pull request May 16, 2018
PR-URL: #20707
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 22, 2018
PR-URL: #20707
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleaxaddaleax mentioned this pull request May 22, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs 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++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

12 participants

@addaleax@apapirovski@mscdex@danbev@jasnell@TimothyGu@cjihrig@tniessen@joyeecheung@BridgeAR@trivikr@nodejs-github-bot