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

build: split up cpplint to avoid long cmd lines #14116

Closed
wants to merge 1 commit into from

Conversation

kfarnung
Copy link
Contributor

@kfarnung kfarnung commented Jul 6, 2017

Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Jul 6, 2017
@kfarnung
Copy link
Contributor Author

kfarnung commented Jul 6, 2017

/cc @refack

Upstream port of nodejs/node-chakracore#330

@refack
Copy link
Contributor

refack commented Jul 6, 2017

Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

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

LGTM, I confirmed that this is runs the same commands as before.

@joaocgreis
Copy link
Member

CI: https://ci.nodejs.org/view/All/job/node-test-pull-request/9048/ (I don't see a way for failures outside test-windows to be related, but every PR must have a full CI run)

refack pushed a commit to refack/node that referenced this pull request Jul 9, 2017
Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

PR-URL: nodejs#14116
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: João Reis <reis@janeasystems.com>
@refack
Copy link
Contributor

refack commented Jul 9, 2017

Landed in d48472c

@refack refack closed this Jul 9, 2017
@refack
Copy link
Contributor

refack commented Jul 9, 2017

addaleax pushed a commit that referenced this pull request Jul 11, 2017
Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

PR-URL: #14116
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: João Reis <reis@janeasystems.com>
@addaleax addaleax mentioned this pull request Jul 11, 2017
@kfarnung kfarnung deleted the cpplint branch July 12, 2017 17:36
addaleax pushed a commit that referenced this pull request Jul 18, 2017
Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

PR-URL: #14116
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

PR-URL: #14116
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: João Reis <reis@janeasystems.com>
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

@kfarnung
Copy link
Contributor Author

@MylesBorins this should not be needed downlevel. The fix was primarily to allow deps (in this case chakrashim) to be able to have their own lint run without exceeding the Windows command line limit.

@refack do you agree?

@refack
Copy link
Contributor

refack commented Aug 16, 2017

Not needed but harmless to backport and even a tiny bit beneficial, if only to reduce disparity between the versions.
@kfarnung if you don't have the spare cycles, I'll do it.

@kfarnung
Copy link
Contributor Author

I'll take care of it today.

kfarnung added a commit to kfarnung/node that referenced this pull request Aug 16, 2017
Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

PR-URL: nodejs#14116
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: João Reis <reis@janeasystems.com>
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
Refactors cpplint slightly to allow multiple runs of it. This allows
downstream projects to run cpplint on their dependencies.

Backport-PR-URL: #14879
PR-URL: #14116
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: João Reis <reis@janeasystems.com>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants