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

Show help on no command #252

Merged

Conversation

itsMapleLeaf
Copy link
Contributor

@itsMapleLeaf itsMapleLeaf commented Jan 26, 2018

What kind of change does this PR introduce?
Shows the full help content when no arguments are given to the CLI, and when the webpack build fails.

Did you add tests for your changes?
Yes.

Summary
See #124

Before, when running the webpack build without a config, the CLI runs webpack with the default config, and fails with an unhelpful, possibly confusing error message. The original issue suggests to show help to solve this issue.

The reasons behind these specific conditions is to allow the default configuration to run as-is without arguments.

Does this PR introduce a breaking change?
No

@itsMapleLeaf itsMapleLeaf changed the title Show help on no command [WIP] Show help on no command Jan 26, 2018
kingdaro added 3 commits January 26, 2018 11:18
makes it so help is shown when the webpack build fails and there is no webpack.config.js

possibly hide the build failing error as well? also consider showing additional information about the default configuration
@itsMapleLeaf itsMapleLeaf changed the title [WIP] Show help on no command Show help on no command Jan 26, 2018
@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@ematipico
Copy link
Contributor

Hi @kingdaro, just noticed now that there's also another PR that tries to fix the same issue. I am confused. But I looked at the changes and it looked different from #254 ! Could you provide a small background inside the description of the PR? Please use the template provided :)

@itsMapleLeaf
Copy link
Contributor Author

itsMapleLeaf commented Jan 29, 2018

Sure, updating it in just a moment.

Done. It looks like they picked up the issue right as I changed my mind and decided to pick it up as well :( Either way, you can merge theirs in and close this if it looks better.

@itsMapleLeaf
Copy link
Contributor Author

Right now in this PR it shows the full help text, but I like the idea from the other PR that instead says "run again with --help for more options" for brevity. That's probably the better approach.

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

I'm going to approve this, and #254 is going to replace it if it gets attention. LGTM :shipit: ty @kingdaro ! 😻 Before we can merge this, we have to make sure it doesn't mess with the 0CJS init

@fokusferit fokusferit merged commit 4cdce07 into webpack:master Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants