Skip to content

Conversation

@refack
Copy link
Contributor

  • Preserve 3 node-core checks
  • Remove TAP to logfile (unused)

Fixes: #25760
Refs: https://github.com/cpplint/cpplint/blob/3d8f6f876dd6e3918e5641483298dbc82e65f358/cpplint.py

/CC @nodejs/build-files @nodejs/python

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-bot
Copy link
Collaborator

@nodejs-github-botnodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jan 28, 2019
@refackrefack self-assigned this Jan 28, 2019
@refackrefack added c++ Issues and PRs that require attention from people who are familiar with C++. python PRs and issues that require attention from people who are familiar with Python. labels Jan 28, 2019
Copy link
Contributor

@thefourtheyethefourtheye left a comment

Choose a reason for hiding this comment

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

Rubber Stamp LGTM

@cclausscclauss mentioned this pull request Jan 28, 2019
4 tasks
@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 28, 2019
Copy link
Member

Choose a reason for hiding this comment

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

Long line.

Copy link
Member

Choose a reason for hiding this comment

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

Indent error (or does this come from upstream?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Google favors is 2 space indentation. There is a pretty good chance that this code was formatted with https://github.com/google/yapf

Copy link
ContributorAuthor

@refackrefackJan 28, 2019

Choose a reason for hiding this comment

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

All white-space changes are lifted verbatim from https://github.com/cpplint/cpplint/blob/3d8f6f876dd6e3918e5641483298dbc82e65f358/cpplint.py

P.S. I'll refactor the commits to make that easier to review

@refack
Copy link
ContributorAuthor

* Preserve 3 node-core checks * Preserve patch to `FileInfo.RepositoryName` * Remove TAP to logfile (unused) PR-URL: nodejs#25771Fixes: nodejs#25760 Refs: https://github.com/cpplint/cpplint/blob/3d8f6f876dd6e3918e5641483298dbc82e65f358/cpplint.py Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@refackrefackforce-pushed the bump-cpplint-3d8f6f876d branch from 6404279 to 4d19300CompareJanuary 30, 2019 23:20
@refackrefack removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 30, 2019
@refackrefack merged commit 4d19300 into nodejs:masterJan 30, 2019
@refackrefack deleted the bump-cpplint-3d8f6f876d branch January 30, 2019 23:28
addaleax pushed a commit that referenced this pull request Feb 1, 2019
addaleax pushed a commit that referenced this pull request Feb 1, 2019
* Preserve 3 node-core checks * Preserve patch to `FileInfo.RepositoryName` * Remove TAP to logfile (unused) PR-URL: #25771Fixes: #25760 Refs: https://github.com/cpplint/cpplint/blob/3d8f6f876dd6e3918e5641483298dbc82e65f358/cpplint.py Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@targostargos mentioned this pull request Feb 14, 2019
@refackrefack removed their assignment Mar 11, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tools: need to upgrade tools/cpp_lint.py to be Python 3 compatible

6 participants

@refack@nodejs-github-bot@bnoordhuis@thefourtheye@cclauss@Trott