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

docs(readme): fix typos, add summary of all commands #845

Merged
merged 3 commits into from
Apr 22, 2019

Conversation

anshumanv
Copy link
Member

fix typos, add summary of all commands in commands section

What kind of change does this PR introduce?
docs

Did you add tests for your changes?
NA

If relevant, did you update the documentation?
Yes

Summary

  • fix some typos/grammars
  • Add a quick summary of all commands in readme.

Does this PR introduce a breaking change?
No

Other information
NA

fix typos, add summary of all commands in commands section
README.md Outdated
@@ -27,7 +27,7 @@
- [How to install](#how-to-install)
* [Getting Started](#getting-started)
* [webpack CLI Scaffolds](#webpack-cli-scaffolds)
* Commands
* [Commands](#commands)
- [`webpack-cli init`](./packages/init/README.md#webpack-cli-init)
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we still keep all list of commands here? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the new one, I would remove this 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing, let's see how everyone else finds it :)

@webpack-bot
Copy link

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

Copy link
Contributor

@misterdev misterdev left a comment

Choose a reason for hiding this comment

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

Well done! Would be nice to have a summary of the other packages like webpack-scaffold, generators and util that aren't mentioned here. What do you think? :)

README.md Outdated
@@ -27,7 +27,7 @@
- [How to install](#how-to-install)
* [Getting Started](#getting-started)
* [webpack CLI Scaffolds](#webpack-cli-scaffolds)
* Commands
* [Commands](#commands)
- [`webpack-cli init`](./packages/init/README.md#webpack-cli-init)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the new one, I would remove this 👍

README.md Outdated Show resolved Hide resolved
remove commands list from the beginning
@webpack-bot
Copy link

@anshumanv Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@misterdev Please review the new changes.

@anshumanv
Copy link
Member Author

@misterdev since (utils, generators, webpack-scaffold) packages are just helpers to other packages and don't have a real command so I'm not sure if it's the best idea to list them in the commands section. WDYT? :)

@misterdev
Copy link
Contributor

Yeah, I would make another list e.g. "Packages" after the "Commands" section :)
Another idea I like is to have a structure like this:

Packages
We organize webpack CLI as a multi-package repository. Every command has a dedicated subfolder in the packages Folder [...]

  • Commands
    Supporting developers is an important task for webpack CLI. Thus, webpack CLI provides different
    commands for many common tasks
    ... list of all commands with description
  • Utilities
    ... list of all other packages with descriptions

IMO, having a complete overview of the repository in the REAME is something desirable

PS: I can work on this, in case it is not in the scope of this PR or you have something else to do :)

include util packages
@anshumanv
Copy link
Member Author

Sounds good @misterdev, just a trivial change. ✨

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.

6 participants