Skip to content

fix: check result when determining exit code of ls <filter> #1986

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

Closed
wants to merge 1 commit into from

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Oct 18, 2020

Alright, take two (because I didn't realise you can't reopen PRs after force pushing 🤦)

I've chucked an assertion to prevent regression into the first filter test - technically they should all ideally do the check, but I've not worked a lot with tap and this is a simple typo bug that I don't expect would crop up all the time.

Happy to add the assertion to the other tests if desired :)

References

Fixes #1979

@G-Rath G-Rath requested a review from a team as a code owner October 18, 2020 19:35
@isaacs
Copy link
Contributor

isaacs commented Oct 20, 2020

This LGTM.

@isaacs isaacs added the Release 7.x work is associated with a specific npm 7 release label Oct 20, 2020
Copy link
Contributor

@isaacs isaacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit that we can definitely squash out at release time if you don't feel like updating and pushing. Otherwise LGTM.

@G-Rath G-Rath force-pushed the fix-empty-ls-results-check branch from c737459 to 434557b Compare October 20, 2020 00:14
@darcyclarke darcyclarke added the release: next These items should be addressed in the next release label Oct 20, 2020
@ruyadorno ruyadorno mentioned this pull request Oct 20, 2020
@ruyadorno ruyadorno closed this in ce4724a Oct 20, 2020
@ruyadorno
Copy link
Contributor

Thanks @G-Rath 🎉 landed in v7.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: next These items should be addressed in the next release Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm ls <pkgname> always exits in error (even when there are no ls problems)
4 participants