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

Check run script existence with undefined #139

Merged
merged 2 commits into from
Feb 18, 2019
Merged

Conversation

vlasy
Copy link
Contributor

@vlasy vlasy commented Jan 18, 2019

Makes it possible to run empty script instead of outputting slightly confusing error message.

In package.json:

"scripts": {
    "empty": ""
  },

Before

> npm run empty
npm ERR! missing script: empty
npm ERR!
npm ERR! Did you mean this?
npm ERR!     empty

npm ERR! A complete log of this run can be found in:
...
>

After

> npm run empty
> 

@vlasy vlasy requested a review from a team as a code owner January 18, 2019 09:07
@aeschright aeschright changed the base branch from latest to release-next January 22, 2019 19:04
@aeschright
Copy link
Contributor

Did this come from a discussion you can point me to? Or can you describe the underlying use case it would solve?

@zkat zkat force-pushed the release-next branch 2 times, most recently from 82170b4 to db63b89 Compare January 22, 2019 23:37
@vlasy
Copy link
Contributor Author

vlasy commented Jan 23, 2019

I was running one of my projects through some automated build tools which needed specific npm scripts to exist. In this project, there was no need for that script to run so I created an empty npm script, hoping it would solve the problem. But then I got the error message shown above.

I could not find any discussion about this situation and the change was rather small, so I went straight to PR.

@zkat zkat added the semver:minor new backwards-compatible feature label Jan 28, 2019
@zkat zkat changed the base branch from release-next to latest January 28, 2019 22:30
Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable allowance -- but I can't really review the changes you made, because this needs to be rebased onto latest. See how "Files changed" says 938 🙃 .

@vlasy
Copy link
Contributor Author

vlasy commented Jan 29, 2019

Rebased :)

@vlasy
Copy link
Contributor Author

vlasy commented Feb 6, 2019

Hi @zkat , is there anything else I can do? PR is rebased now :)

Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

Everything looks good now! Thanks a bunch. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor new backwards-compatible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants