Skip to content

Conversation

targos
Copy link
Member

@targos targos commented May 1, 2020

  • Remove actions/setup-node from lint job

There's already a version of Node.js installed on the action runners so
we do not need to explicitly install another one.

  • Add Node.js 14.x to the test matrix

  • Remove CI: true from the environment

The variable is already set by GitHub

@richardlau
Copy link
Member

richardlau commented May 1, 2020

* Remove actions/setup-node from lint job

There's already a version of Node.js installed on the action runners so
we do not need to explicitly install another one.

The action also installs problem matchers for eslint which will annotate lint failures and mark them inline in the "Files changed" tab for pull requests.

@codecov-io
Copy link

codecov-io commented May 1, 2020

Codecov Report

Merging #805 into master will increase coverage by 0.85%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #805      +/-   ##
==========================================
+ Coverage   95.42%   96.27%   +0.85%     
==========================================
  Files          26       30       +4     
  Lines         875      887      +12     
==========================================
+ Hits          835      854      +19     
+ Misses         40       33       -7     
Impacted Files Coverage Δ
test/fixtures/omg-i-write-to-tmpdir/test.js 100.00% <100.00%> (ø)
test/fixtures/omg-i-pass-with-scripts/build.js 100.00% <0.00%> (ø)
...ixtures/omg-i-pass-with-install-param/test_args.js 100.00% <0.00%> (ø)
test/fixtures/omg-i-pass-with-scripts/test.js 100.00% <0.00%> (ø)
bin/citgm-all.js 92.00% <0.00%> (+1.00%) ⬆️
bin/citgm.js 100.00% <0.00%> (+2.00%) ⬆️
lib/match-conditions.js 93.61% <0.00%> (+2.12%) ⬆️
lib/spawn.js 100.00% <0.00%> (+30.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b925780...b187a54. Read the comment docs.

@targos
Copy link
Member Author

targos commented May 1, 2020

* Remove actions/setup-node from lint job

There's already a version of Node.js installed on the action runners so
we do not need to explicitly install another one.

The action also installs problem matchers for eslint which will annotate lint failures and mark them inline in the "Files changed" tab for pull requests.

Ok, I'll add it back and use v14.x 👍

@targos
Copy link
Member Author

targos commented May 1, 2020

It looks like the tests do not pass on v14.x. Should we add citgm to citgm ??? 😆

- Use Node.js 14 in actions/setup-node

- Add Node.js 14.x to the test matrix

- Remove CI: true from the environment

The variable is already set by GitHub

- Fix bug in test fixture

Discovered with v14.x because of
nodejs/node#31030.
@targos
Copy link
Member Author

targos commented May 1, 2020

There was a bug in our fixtures. I fixed it.
Refs: nodejs/node#31030

@targos targos merged commit a67edc7 into nodejs:master May 2, 2020
@targos targos deleted the update-action branch May 2, 2020 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants