-
Notifications
You must be signed in to change notification settings - Fork 397
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
pre-commit autoupdate #501
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #501 +/- ##
=======================================
Coverage 92.24% 92.24%
=======================================
Files 91 91
Lines 11033 11033
Branches 1524 1524
=======================================
Hits 10177 10177
Misses 851 851
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Changes produced by `pre-commit autoupdate`. Signed-off-by: Christian Clauss <[email protected]>
Signed-off-by: Christian Clauss <[email protected]>
6e04372
to
f4204bc
Compare
Whoops, both of us made a change at the same time there. @cclauss Why the downgrade of |
https://github.com/bloomberg/memray/actions/runs/6984049688/job/19006277652 |
Ah, "prettier requires at least version 14 of Node, please upgrade". Hm. But we're installing Node 16...
Hm. I guess pre-commit isn't picking up the version that we installed... |
I tend to use https://pre-commit.ci instead of GHA to run pre-commit. |
59d2d00
to
bacdd51
Compare
It seems that it wants to pick up a binary called `nodejs`, while `actions/setup-node` is only installing one called `node`. Let's hack around this with a symlink. Signed-off-by: Matt Wozniski <[email protected]>
bacdd51
to
e9bc3d7
Compare
I think I've got it. And It seems we can hack around it by symlinking the |
I've entered actions/setup-node#905 for that since that doesn't seem like expected behavior (though I'm not a JS person so it's entirely possible this is a nodeenv problem and not an actions/setup-node problem... we'll see) |
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.
Well, my terrible hack works well enough, so LGTM. Thanks @cclauss!
Issue number of the reported bug or feature request: #
Describe your changes
A clear and concise description of the changes you have made.
%
pre-commit autoupdate
Testing performed
Describe the testing you have performed to ensure that the bug has been addressed, or that the new feature works as planned.
%
pre-commit run --all-files
Additional context
Add any other context about your contribution here.