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.

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

tools: edit cpplint.py #35516

Closed
wants to merge 2 commits into from
Closed

tools: edit cpplint.py #35516

wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 6, 2020

  • Remove unnecessary setting of classinfo where it is set again before
    it is used.
  • Change whitelisted_functions to allowed_functions.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

* Remove unnecessary setting of classinfo where it is set again before
  it is used.
* Change whitelisted_functions to allowed_functions.
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Oct 6, 2020
tools/cpplint.py Outdated Show resolved Hide resolved
@richardlau
Copy link
Member

Are we ever likely to sync this with upstream? The last time we did was #27098 and it looks like whitelisted_functions is gone from https://github.com/cpplint/cpplint via cpplint/cpplint@08268ef / cpplint/cpplint@64de843.

@Trott
Copy link
Member Author

Trott commented Oct 6, 2020

Are we ever likely to sync this with upstream? The last time we did was #27098 and it looks like whitelisted_functions is gone from https://github.com/cpplint/cpplint via cpplint/cpplint@08268ef / cpplint/cpplint@64de843.

It's also gone from https://github.com/google/styleguide/blob/gh-pages/cpplint/cpplint.py, if we want to sync from that upstream instead. (I don't know the relationship between the two other than that one is a fork of the other on GitHub.)

@Trott
Copy link
Member Author

Trott commented Oct 6, 2020

The superfluous classinfo assignment still appears in the upstream fork, but I'm happy to submit the PR for that there if that's the better way to go.

@richardlau
Copy link
Member

Are we ever likely to sync this with upstream? The last time we did was #27098 and it looks like whitelisted_functions is gone from https://github.com/cpplint/cpplint via cpplint/cpplint@08268ef / cpplint/cpplint@64de843.

It's also gone from https://github.com/google/styleguide/blob/gh-pages/cpplint/cpplint.py, if we want to sync from that upstream instead. (I don't know the relationship between the two other than that one is a fork of the other on GitHub.)

Me neither, but whatever we decide to sync from I'd like to keep the number of floating patches we have to maintain to a minimum.

@cclauss
Copy link
Contributor

cclauss commented Oct 6, 2020

My vote would be https://github.com/cpplint/cpplint -- I have made contributions there in the past and it has gone quite smoothly. It is 300+ commits ahead of the other and only 7 commits behind.

@Trott
Copy link
Member Author

Trott commented Oct 9, 2020

This will probably be supplanted by #35569 and a follow-on PR to that, so I'll move this to draft for now, and close it when the relevant changes have landed to make this obsolete.

@Trott Trott marked this pull request as draft October 9, 2020 13:06
@Trott Trott closed this Dec 11, 2020
@Trott Trott deleted the cpplint-classinfo branch December 11, 2020 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants