-
Notifications
You must be signed in to change notification settings - Fork 18
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
@puku0x Thank you for creating the PR! Can you check my review, please?
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.
@puku0x Thanks for the quick fix! In addition, could you please do the following if you have time?
- Simplify the
lint
job removing the matrix and specifyinglts/*
. - Remove needless
env.CI
because Actions adds it by default
@ybiquitous Sure! I thought the same thing about |
It seems |
Let's remove the matrix from the |
Done! |
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.
@puku0x Thank you so much! I've left a trivial suggestion, but this PR looks good to me! 👍🏼
Co-authored-by: Masafumi Koba <[email protected]>
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.
Thank you so much for your contribution! 😊
None, as it's a CI fix.
Prefer the latest one, as it is recommended.npm
should respect its supported Node.js version.actions/setup-node@v2
supports~/.npm
cache.