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.

Sign up for GitHub

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

Jump to bottom

doc: link to code style guide #12636

Closed
refack opened this issue Apr 25, 2017 · 12 comments
Closed

doc: link to code style guide #12636

refack opened this issue Apr 25, 2017 · 12 comments
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. wip Issues and PRs that are still a work in progress.

Comments

@refack
Copy link
Contributor

refack commented Apr 25, 2017 •
edited
Loading

  • Subsystem: doc

IMHO we should add a link to the Google Style Guide according to which me are linting C++
and add some Best Practices

After #12540 I thought we should add something like:

When editing C++

If you add a new macro or use a new external function, make sure you include the header file explicitly in the file you are editing. This makes sure your change is independent of previous changes, and makes it easier to backport
@refack refack added doc Issues and PRs related to the documentations. question Issues that look for answers. labels Apr 25, 2017
@gibfahn
Copy link
Member

gibfahn commented Apr 25, 2017

I think in general we try to document our coding style in lint rules. I don't know how easy it would be to lint for this.

@refack
Copy link
Contributor Author

refack commented Apr 25, 2017

Yeah I was thinking about that (linting for header include)

@refack
Copy link
Contributor Author

refack commented Apr 25, 2017

Still maybe we should have a style-guide to save new devs one "code" -> "lint" -> "fix" cycle

@refack refack mentioned this issue Apr 25, 2017
Closed
2 tasks
@gibfahn
Copy link
Member

gibfahn commented Apr 25, 2017

Still maybe we should have a style-guide to save new devs one "code" -> "lint" -> "fix" cycle

The problem is that you then have to keep the style guide up to date with the linter. And I'm pretty sure the average dev would rather run the linter for each project than read the style guide, especially when you can just pass --fix to autofix basic issues.

@refack
Copy link
Contributor Author

refack commented Apr 25, 2017

So I've been reading cpplint.py, and was reminded nodejs/node-v0.x-archive#8487 that it actualy linting acording to https://google.github.io/styleguide/cppguide.html, so may be a link, and some rules of thumb?

@refack refack changed the title doc: code style guide doc: link to code style guide Apr 25, 2017
@refack refack removed the question Issues that look for answers. label Apr 25, 2017
@refack
Copy link
Contributor Author

refack commented Apr 27, 2017

Ref: #11753

This was referenced Apr 29, 2017
Closed
Closed
@Trott
Copy link
Member

Trott commented Aug 4, 2017

This should remain open? If so, maybe label help wanted?

@refack refack added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. mentor-available meta Issues and PRs related to the general management of the project. labels Aug 12, 2017
@Divya063
Copy link

@refack I would like to work on this

@refack
Copy link
Contributor Author

refack commented Oct 12, 2017

@Divya063 there is already some work in progress to do that (#16090). Sorry I didn't update this issue.

@refack refack added wip Issues and PRs that are still a work in progress. and removed help wanted Issues that need assistance from volunteers or PRs that need help to proceed. mentor-available labels Oct 12, 2017
@refack refack mentioned this issue Oct 12, 2017
Closed
@Divya063
Copy link

@refack no problem. Is there other beginners-friendly issue on which I can work?

@Trott
Copy link
Member

Trott commented Oct 12, 2017

Hi, @Divya063! Take a look at http://nodetodo.org/getting-started/ for one way to get a good first task to work on in Node.js core.

@Divya063
Copy link

@Trott Thanks

addaleax added a commit to ayojs/ayo that referenced this issue Oct 15, 2017
@addaleax
doc: add basic C++ style guide
3397432 
Ideally, most of these things would be enforced via linter rules.
This is a first step into having a style guide that goes beyond
what the linter currently enforces.

PR-URL: nodejs/node#16090
Fixes: nodejs/node#12636
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos pushed a commit that referenced this issue Oct 18, 2017
@addaleax @targos
doc: add basic C++ style guide
3266e1f 
Ideally, most of these things would be enforced via linter rules.
This is a first step into having a style guide that goes beyond
what the linter currently enforces.

PR-URL: #16090
Fixes: #12636
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. wip Issues and PRs that are still a work in progress.
Projects
None yet
Milestone
No milestone
4 participants
@refack @Trott @gibfahn @Divya063