Skip to content
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

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

shadowspawn
Copy link
Collaborator

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:

@ljharb
Copy link
Member

ljharb commented Jan 6, 2023

I suppose this is fine, but I have this same thing on 300+ other projects and it's not much of a problem ¯\_(ツ)_/¯

@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2023

Codecov Report

Merging #20 (ba92fe6) into main (950eaa7) will not change coverage.
The diff coverage is n/a.

📣 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.

@shadowspawn
Copy link
Collaborator Author

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!

@ljharb
Copy link
Member

ljharb commented Jan 6, 2023

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 :-/

@shadowspawn
Copy link
Collaborator Author

That's what optional vs required is for :-) failing optional test are expected, failing required tests are a problem.

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.

Screenshot 2023-01-09 at 19 50 43

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Feb 9, 2023

(Woop! Thanks. I appreciate you are willing to do something different than your other projects. 👍 )

@ljharb
Copy link
Member

ljharb commented Feb 9, 2023

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 :-)

@ljharb ljharb merged commit ba92fe6 into minimistjs:main Feb 9, 2023
@shadowspawn shadowspawn deleted the feature/skip-0.6-tests branch February 19, 2023 00:25
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.

GitHub action checks on PR always show failure
3 participants