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(packages): add missing READMES to packages #545

Merged
merged 1 commit into from
Jul 18, 2018

Conversation

evenstensberg
Copy link
Member

What kind of change does this PR introduce?
Enhancement

Did you add tests for your changes?
N/A
If relevant, did you update the documentation?
Yes
Summary
Adds README files to all our mono repo packages

Does this PR introduce a breaking change?
no

Other information
=)

@webpack-bot
Copy link

@ev1stensberg The most important CI builds failed. This way your PR can't be merged.

Please take a look at the CI results from travis (failure) and fix these issues.

@evenstensberg evenstensberg changed the base branch from next to master July 18, 2018 02:15
@evenstensberg evenstensberg merged commit a2f10e9 into master Jul 18, 2018
@evenstensberg evenstensberg deleted the feat/package-readmes branch July 18, 2018 02:15
@fokusferit
Copy link
Contributor

But Ci failed? 😅

@ematipico
Copy link
Contributor

And not reviewed 🤔

@evenstensberg
Copy link
Member Author

CI is @dhruvdutt s task

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Overall looks fine but the commands are all mixed, which could lead to confusion to the end user. We should cover always local and global usage/installation

@@ -2,7 +2,7 @@

## Description

This package contains the logic to add new properties to the a webpack configuration file. It will run a generator that prompts the user for questions of which property to add to their webpack configuration file.
This package contains the logic to add new properties in a webpack configuration file. It will run a generator that prompts the user for questions of which property to add to their webpack configuration file.
Copy link
Contributor

Choose a reason for hiding this comment

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

the user some question about which properties to add


## Description

This package contains the logic to initiate new loader projects.
Copy link
Contributor

Choose a reason for hiding this comment

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

initiate => create
Simpler wording


### CLI (via `webpack-cli`)
```bash
npx webpack-cli generate-loader
Copy link
Contributor

Choose a reason for hiding this comment

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

We should always split installation and usage of the packages in local and global:

npx webpack-cli generate-loader

and

webpack-cli generate-loader

In this file we mixed, which is confusing for the user


## Description

This package contains the logic to initiate new plugin projects.
Copy link
Contributor

Choose a reason for hiding this comment

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

initiate => create


## Description

This package contains all webpack-cli related yeoman generators.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should explain what the generators do


## Description

This package contains the logic to remove properties of a webpack configuration file. It will run a generator that prompts the user for questions of which property to remove in their webpack configuration file.
Copy link
Contributor

Choose a reason for hiding this comment

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

the user some question about which

@ematipico
Copy link
Contributor

@ev1stensberg left some comments about some changes. Unfortunately this is already been merged (don't know why).

@evenstensberg
Copy link
Member Author

Submit a PR with patch label @ematipico

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.

4 participants