FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Upgrade cpplint by bnoordhuis · Pull Request #7462 · nodejs/node · GitHub

/ node Public

Upgrade cpplint - #7462

Merged
bnoordhuis merged 22 commits into
nodejs:masterfrom
bnoordhuis:upgrade-cpplint
Jun 29, 2016
ConversationCommits (22)ChecksFiles changed
Merged

Upgrade cpplint#7462
bnoordhuis merged 22 commits into
nodejs:masterfrom
bnoordhuis:upgrade-cpplint

Conversation

bnoordhuis commented Jun 28, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #7444.

I dropped our custom whitespace/if-one-line rule because I don't think it's all that great and didn't feel like reimplementing it.

R=@trevnorris?

CI: https://ci.nodejs.org/job/node-test-pull-request/3109/

bnoordhuis added 22 commits June 28, 2016 21:21
tools: update cpplint to r456
04c723d 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
tools: disable unwanted cpplint rules again
fc10624 
This commit disables the build/include, build/include_alpha,
build/include_order and legal/copyright warnings again.

PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
tools: add back --mode=tap to cpplint
bb81d69 
This commit reimplements commit 7b45163 ("tools: add tap output to
cpplint") on top of the upgraded copy of cpplint.

PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
tools: allow cpplint to run outside git repo
fadf66a 
This reapplies commit a493dab ("cpplint: make it possible to run
outside git repo") from September 2015, this time with a proper
status line.

PR-URL: nodejs#7462
Refs: nodejs#2693
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix build/c++tr1 cpplint warnings
19ec5d7 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix build/header_guard cpplint warnings
e9559d0 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix readability/braces cpplint warnings
29228c4 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix readability/constructors cpplint warnings
df3a192 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix readability/inheritance cpplint warnings
8e39522 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix readability/namespace cpplint warnings
0ba0eca 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix readability/nolint cpplint warnings
d112df4 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix runtime/indentation_namespace warnings
1243134 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix runtime/int cpplint warnings
ae0c650 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix runtime/references cpplint warnings
2f2002a 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix whitespace/blank_line cpplint warnings
0daffdf 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: fix whitespace/indent cpplint warnings
97aeb0a 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
build: remove unused files from CPPLINT_FILES
f52e26f 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: lint node_win32_perfctr_provider.cc
a26c3cd 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: lint node_lttng_tp.h
f1d2792 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: lint v8abbr.h
dbe4b58 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
tools: fix -Wunused-variable warning
9a6439a 
PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
src: remove obsolete NOLINT comments
f8d3f6f 
Obsoleted by the recent cpplint upgrade.

PR-URL: nodejs#7462
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
bnoordhuis added c++ Issues and PRs that require attention from people who are familiar with C++. build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jun 28, 2016
nodejs-github-bot added the inspector Issues and PRs related to the V8 inspector protocol label Jun 28, 2016
bnoordhuis mentioned this pull request Jun 28, 2016
Closed
mscdex removed the inspector Issues and PRs related to the V8 inspector protocol label Jun 28, 2016
trevnorris reviewed Jun 28, 2016
View reviewed changes
Comment thread src/node_version.h Outdated

trevnorris Jun 28, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

heh. this rule was the first thing I spent time fixing on my initial attempt. wish I had taken the time to see how many lines of code were affected first.

Fishrock123 commented Jun 29, 2016

Copy link
Copy Markdown
Contributor

@bnoordhuis Looks like there might have been a merge problem here? https://github.com/nodejs/node/commits/9a6439aaccd7e712a5543acdae8af5909aa91c7c shows commits also ontop of it.

Also 9beef23...fba271b shows your commits alongside #6436

Fishrock123 mentioned this pull request Jun 29, 2016
Closed
3 tasks

addaleax commented Jun 29, 2016

Copy link
Copy Markdown
Member

The history should be fine, the commits from this PR just have weird-ish CommitDate: fields that weren’t updated during rebasing…?

Fishrock123 commented Jun 29, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Maybe the issue was the commits from #7016? They don't seem to be based on these commits yet come after these commits?

bnoordhuis commented Jun 29, 2016

Copy link
Copy Markdown
Member Author

I did a git filter-branch --env-filter on them with a GIT_COMMITTER_DATE set in the past, that's probably why they show up like that.

Fishrock123 commented Jun 29, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

@bnoordhuis May I ask why? (Why you did that lol)

bnoordhuis commented Jun 29, 2016

Copy link
Copy Markdown
Member Author

I wanted to update the author date with GIT_AUTHOR_DATE but I also had GIT_COMMITTER_DATE set. Not normally an issue but I think GH sorts by timestamp rather than parent commit.

Fishrock123 commented Jun 29, 2016

Copy link
Copy Markdown
Contributor

... strange. I guess it probably sorts like that because of regular merges.

Fishrock123 mentioned this pull request Jul 5, 2016
Merged

MylesBorins commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

@bnoordhuis as you can imagine 9beef23...f8d3f6f is not landing cleanly on v4.x. Would you be willing to do a backport?

bnoordhuis commented Jul 11, 2016

Copy link
Copy Markdown
Member Author

I'll do it when the current merge storm has died down a bit, I don't want to have to rebase it over and over.

MylesBorins commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

totally reasonable. I'll likely finish up the merge storm by EOD today

addaleax commented Jul 12, 2016

Copy link
Copy Markdown
Member

Backport is in #7666

MylesBorins mentioned this pull request Jul 12, 2016
Merged
joshgav mentioned this pull request Nov 1, 2016
Closed
2 tasks
cclauss mentioned this pull request Jan 28, 2019
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL