-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: add GitHub actions to run linters #31323
Conversation
Add jobs to our GitHub Actions workflow to run our various lint Makefile targets. The `setup-node` action used to run the JavaScript linter contains problem matchers for eslint that will annotate the files in a pull request if there are failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
The CI workflow doesn't use the matrix strategy so there is no `${{ matrix.node-version }}` variable to use in the `Use Node.js` step names.
- name: Use Node.js ${{ matrix.node-version }} | ||
uses: actions/setup-node@v1 | ||
with: | ||
node-version: 10.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to use 12.x. It should be faster and will be supported for a longer time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deliberately went for the earliest in support version of Node.js on the basis that it would alert us if any of our tooling involved in the linting would break on it and thus help apply dont-land-on
labels (with an assumption that its very unlikely that something would break on 12 but not 10).
As for speed the lint checks are completing in less than 2 minutes which is a fraction of the time taken to run the build jobs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to have such guarantees, we could even let it run on both versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally in the long term when actions/setup-node#26 is addressed we can put an alias in here like lts
if the concern is that the version here will eventually need to be updated.
Since this file is not exercised or even linted in Jenkins CI, green GitHub actions and Travis should be sufficient to land. |
Landed in f4a4a1a...b772b9a |
Add jobs to our GitHub Actions workflow to run our various lint Makefile targets. The `setup-node` action used to run the JavaScript linter contains problem matchers for eslint that will annotate the files in a pull request if there are failures. PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The CI workflow doesn't use the matrix strategy so there is no `${{ matrix.node-version }}` variable to use in the `Use Node.js` step names. PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add jobs to our GitHub Actions workflow to run our various lint Makefile targets. The `setup-node` action used to run the JavaScript linter contains problem matchers for eslint that will annotate the files in a pull request if there are failures. PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The CI workflow doesn't use the matrix strategy so there is no `${{ matrix.node-version }}` variable to use in the `Use Node.js` step names. PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add jobs to our GitHub Actions workflow to run our various lint Makefile targets. The `setup-node` action used to run the JavaScript linter contains problem matchers for eslint that will annotate the files in a pull request if there are failures. PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The CI workflow doesn't use the matrix strategy so there is no `${{ matrix.node-version }}` variable to use in the `Use Node.js` step names. PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add jobs to our GitHub Actions workflow to run our various lint Makefile targets. The `setup-node` action used to run the JavaScript linter contains problem matchers for eslint that will annotate the files in a pull request if there are failures. PR-URL: nodejs#31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The CI workflow doesn't use the matrix strategy so there is no `${{ matrix.node-version }}` variable to use in the `Use Node.js` step names. PR-URL: nodejs#31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add jobs to our GitHub Actions workflow to run our various lint Makefile targets. The `setup-node` action used to run the JavaScript linter contains problem matchers for eslint that will annotate the files in a pull request if there are failures. Backport-PR-URL: #32608 PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The CI workflow doesn't use the matrix strategy so there is no `${{ matrix.node-version }}` variable to use in the `Use Node.js` step names. Backport-PR-URL: #32608 PR-URL: #31323 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add jobs to our GitHub Actions workflow to run our various lint Makefile
targets. The
setup-node
action used to run the JavaScript linter containsproblem matchers for eslint that will annotate the files in a pull request if
there are failures.
The Python linting has been left out as I believe it's being included as part of
#29474.
See https://github.com/richardlau/node-1/pull/3/files for an example of the
annotations for lint-js failures.
We may be able to create problem matchers for the cpplint.py and markdown
linters in the future based on
https://github.com/actions/toolkit/blob/master/docs/problem-matchers.md.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes