-
-
Notifications
You must be signed in to change notification settings - Fork 36
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
Avoid 0.6 tests due to build failures #20
Conversation
I suppose this is fine, but I have this same thing on 300+ other projects and it's not much of a problem ¯\_(ツ)_/¯ |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #20 +/- ##
=======================================
Coverage 98.75% 98.75%
=======================================
Files 1 1
Lines 161 161
Branches 71 71
=======================================
Hits 159 159
Misses 2 2 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I really dislike failing tests. I feel either it is worth fixing now, or better as an open issue for long-term problems. Tracking ToDos or short-comings by failing tests is noisy and detracting from the checks, and in particular from my point of view it is mental overhead for people not familiar with the underlying issue. I am more willing to accept noise from my own failing tests that I intend to fix, so I have some sympathy with your comment! |
That's what optional vs required is for :-) failing optional test are expected, failing required tests are a problem. I am more than happy to see a fix in the action itself, but getting node 0.6 to compile on GHA is sadly not a simple task :-/ |
My complaint is identifying the difference. The optional/required does affect the email notifications, but I'm finding it hard to tell the difference in the web UX. Here are three PR. One passes all tests, one has a failing "required" test, and one has a failing "optional" test. |
95efa4c
to
245119c
Compare
245119c
to
ba92fe6
Compare
(Woop! Thanks. I appreciate you are willing to do something different than your other projects. 👍 ) |
Realistically this is a compelling argument to disable it in the shared action, until I can get it working, but I'll merge this now to make it easier on you in the meantime :-) |
Node.js 0.6 does not install and PR checks always fail . Expand range to avoid the failing installation.
Fixes #19
An alternative would be to disable 0.6 upstream, or fix! I'm opening a PR for a local solution/work-around.
I could open another issue in minimist to reenable 0.6 if/when fixed upstream? Reference: