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

tools: update cpplint.py #8487

Closed
wants to merge 1 commit into from
Closed

tools: update cpplint.py #8487

wants to merge 1 commit into from

Conversation

refack
Copy link
Contributor

@refack refack commented Oct 2, 2014

  • filter out new errors for now

@indutny
Copy link
Member

indutny commented Oct 4, 2014

Could you please justify this?

@refack
Copy link
Contributor Author

refack commented Oct 4, 2014

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

@indutny
Copy link
Member

indutny commented Oct 4, 2014

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

@refack
Copy link
Contributor Author

refack commented Oct 4, 2014

New warnings:
https://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
Copy link
Contributor Author

refack commented Oct 10, 2014

@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://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]

- filter out new errors for now

Conflicts:
	Makefile
	src/smalloc.cc
@jasnell
Copy link
Member

jasnell commented Aug 13, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants