doc,meta: add references to outside C++ guides #23317

refack · 2018-10-07T17:16:56Z

Suggested words about outside coding guidelines, and set their priorities.
Fixes: #22862

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-10-07T17:16:57Z

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1158/pipeline

joyeecheung · 2018-10-07T20:40:29Z

CPP_STYLE_GUIDE.md

Somehow I feel like this should be Semantics and Memory Management can be its own section?
(Also ideally we can have a section dedicated to C++/JS interactions and V8/libuv usage)

joyeecheung · 2018-10-07T20:43:08Z

CPP_STYLE_GUIDE.md

It's a bit odd to put this section here...can we move it to the beginning of the document? We do inherit some formatting styles from Google's C++ style anyway.

Ack x 2.
I'll try to reformat this.

joyeecheung · 2018-10-07T20:44:30Z

CPP_STYLE_GUIDE.md

I wouldn't mention clang-tidy as a goal here since the last time I attempted to use it on our code base it took really long to complete linting...certainly much longer than cpplint and cpplint is already kind of slow.

Ack.
Recent versions seems to work much much better. MSVS has it built in, and it runs in reasonable time (for just /src/ and with a limited ruleset). It does spit out a huge amount of recommendations.

joyeecheung · 2018-10-07T20:45:26Z

CPP_STYLE_GUIDE.md

s/ATM/At the moment/

automatic teller machine*

addaleax · 2018-10-07T23:11:32Z

CPP_STYLE_GUIDE.md

If you want, you can remove this paragraph or merge it into the previous one, since it shares a lot of substance with it.

Eventually, I decided to keep it as is. I think it's better to keep semantic from formatting separate, even in this case.

addaleax · 2018-10-07T23:12:35Z

CPP_STYLE_GUIDE.md

I’d prefer to spell out the abbreviations.

Also, maybe make it a bit clearer that our style guide is closest to the Google one?

vsemozhetbyt · 2018-10-08T19:55:18Z

CPP_STYLE_GUIDE.md

based of -> based on?

vsemozhetbyt · 2018-10-08T19:56:41Z

CPP_STYLE_GUIDE.md

A space between the link and (GCSG)?

vsemozhetbyt · 2018-10-08T19:56:53Z

CPP_STYLE_GUIDE.md

with explicit priorities PR-URL: nodejs#23317 Reviewed-By: Anna Henningsen <[email protected]>

refack · 2018-10-13T22:08:46Z

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1237/

addaleax · 2018-10-13T22:21:33Z

@refack In case you missed it: #22255

refack · 2018-10-13T22:24:08Z

It's a week old.
Ohh, it's just 6 days, my bad.

with explicit priorities PR-URL: #23317 Reviewed-By: Anna Henningsen <[email protected]>

refack added c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. discuss Issues opened for discussions and feedbacks. meta Issues and PRs related to the general management of the project. labels Oct 7, 2018

refack self-assigned this Oct 7, 2018

refack requested review from addaleax and joyeecheung October 7, 2018 17:16

refack mentioned this pull request Oct 7, 2018

src: reduce scope of variables throughout codebase #23297

Merged

2 tasks

joyeecheung reviewed Oct 7, 2018

View reviewed changes

refack force-pushed the cpp-guideline-references branch from 5e72e11 to 546a42f Compare October 7, 2018 23:08

addaleax approved these changes Oct 7, 2018

View reviewed changes

vsemozhetbyt reviewed Oct 8, 2018

View reviewed changes

doc,meta: add references to outside C++ guides

0f8eaa4

with explicit priorities PR-URL: nodejs#23317 Reviewed-By: Anna Henningsen <[email protected]>

refack force-pushed the cpp-guideline-references branch from 4f9a027 to 0f8eaa4 Compare October 13, 2018 22:08

refack removed the discuss Issues opened for discussions and feedbacks. label Oct 13, 2018

refack merged commit 0f8eaa4 into nodejs:master Oct 13, 2018

refack deleted the cpp-guideline-references branch October 13, 2018 22:15

addaleax pushed a commit that referenced this pull request Oct 14, 2018

doc,meta: add references to outside C++ guides

a0c8f85

with explicit priorities PR-URL: #23317 Reviewed-By: Anna Henningsen <[email protected]>

jasnell pushed a commit that referenced this pull request Oct 17, 2018

doc,meta: add references to outside C++ guides

cabf144

with explicit priorities PR-URL: #23317 Reviewed-By: Anna Henningsen <[email protected]>

refack removed their assignment Oct 20, 2018

MylesBorins pushed a commit that referenced this pull request Oct 30, 2018

doc,meta: add references to outside C++ guides

2f51e49

with explicit priorities PR-URL: #23317 Reviewed-By: Anna Henningsen <[email protected]>

refack changed the title ~~[WIP] doc,meta: add references to outside C++ guides~~ doc,meta: add references to outside C++ guides Nov 13, 2018

refack mentioned this pull request Nov 13, 2018

c++: Adopt the CppCoreGuidelines #22862

Closed

codebytere mentioned this pull request Nov 27, 2018

v10.13.1 proposal #24675

Closed

rvagg pushed a commit that referenced this pull request Nov 28, 2018

doc,meta: add references to outside C++ guides

8d3af46

with explicit priorities PR-URL: #23317 Reviewed-By: Anna Henningsen <[email protected]>

MylesBorins pushed a commit that referenced this pull request Nov 29, 2018

doc,meta: add references to outside C++ guides

04f27aa

with explicit priorities PR-URL: #23317 Reviewed-By: Anna Henningsen <[email protected]>

codebytere mentioned this pull request Nov 29, 2018

v10.14.2 proposal #24727

Merged

Uh oh!

doc,meta: add references to outside C++ guides #23317

doc,meta: add references to outside C++ guides #23317

Uh oh!

Conversation

refack commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

nodejs-github-bot commented Oct 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

refack commented Oct 13, 2018

Uh oh!

addaleax commented Oct 13, 2018

Uh oh!

refack commented Oct 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

refack commented Oct 7, 2018 •

edited

Loading