Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

tools: update cpplint.py - #8487

Closed
refack wants to merge 1 commit into
nodejs:v0.12from
node4good:up-cpplint
Closed

tools: update cpplint.py#8487
refack wants to merge 1 commit into
nodejs:v0.12from
node4good:up-cpplint

Conversation

@refack

@refack refack commented Oct 2, 2014

Copy link
Copy Markdown
Contributor
  • filter out new errors for now

@indutny

indutny commented Oct 4, 2014

Copy link
Copy Markdown
Member

Could you please justify this?

@refack

refack commented Oct 4, 2014

Copy link
Copy Markdown
Contributor Author

New version of cpplint same one used by v8 2.30.
Includes c++11 support.
Fixing the new warnings is a WIP.

@indutny

indutny commented Oct 4, 2014

Copy link
Copy Markdown
Member

What are the new warnings, and why should we ignore them?

@refack

refack commented Oct 4, 2014

Copy link
Copy Markdown
Contributor Author

New warnings:
https://github.com/proxy/gist.githubusercontent.com/refack/774a677868de42757b57/raw/c9dafb55e64a0e81ca0d6899f3ea43a564f66323/new%20cpplint%20warnings
Shouldn't ignore anything, but fixing might have conflicted with the work on #8476 so I added suppression for now and a TODO mark, to be addressed later.

@refack

refack commented Oct 10, 2014

Copy link
Copy Markdown
Contributor Author

@indutny do you think we (i.e. I) should address new style rules?
I think that the following list is a good place to start.
https://github.com/proxy/gist.githubusercontent.com/refack/774a677868de42757b57/raw/06bec9d97bd51ab7563071ac62e7cfac3320a112/CPPLint%20short%20list

Leaving [build/*] rules for further discussion, i.e.:

  • Include the directory when naming .h files [build/include] [4]
  • Found C system header after other header. Should be: node_stat_watcher.h, c system, c++ system, other. [build/include_order] [4]

Refael Ackermann @refack
tools: update cpplint.py
4d95fd1 
- filter out new errors for now

Conflicts:
	Makefile
	src/smalloc.cc
@jasnell

jasnell commented Aug 13, 2015

Copy link
Copy Markdown
Member

@indutny @orangemocha @misterdjules ... do any of you see a reason to keep this one open?

@refack refack closed this Aug 13, 2015
@refack refack mentioned this pull request Apr 25, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

pr v0.12

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@refack @indutny @jasnell @Nodejs-Jenkins