Skip to content

docs(packages): add missing READMES to packages#545

Merged
evenstensberg merged 1 commit intomasterfrom
feat/package-readmes
Jul 18, 2018
Merged

docs(packages): add missing READMES to packages#545
evenstensberg merged 1 commit intomasterfrom
feat/package-readmes

Conversation

@evenstensberg
Copy link
Copy Markdown
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
Copy Markdown

@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
Copy Markdown
Contributor

But Ci failed? 😅

@ematipico
Copy link
Copy Markdown
Contributor

And not reviewed 🤔

@evenstensberg
Copy link
Copy Markdown
Member Author

CI is @dhruvdutt s task

Copy link
Copy Markdown
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

Comment thread packages/add/README.md
## 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Comment thread packages/remove/README.md

## 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
Copy Markdown
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
Copy Markdown
Contributor

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

@evenstensberg
Copy link
Copy Markdown
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