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

V2 docs/remove commit validator #222

Merged
merged 1 commit into from
Dec 20, 2017
Merged

V2 docs/remove commit validator #222

merged 1 commit into from
Dec 20, 2017

Conversation

peter-mouland
Copy link
Contributor

What kind of change does this PR introduce?
Update Documentation for v2

Did you add tests for your changes?
n/a

If relevant, did you update the documentation?
yes

Summary
Removed any mention of commit-validator as it is no longer relevant.

I did see a task prepare is this a prepublish task? or should the docs now mention this?

Does this PR introduce a breaking change?

n/a

Other information

@peter-mouland peter-mouland changed the base branch from master to v2 December 20, 2017 13:52
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.

Nice! Thanks! Yeah, if you could add npm run prepare / yarn prepare to the docs, that would be great!

.gitignore Outdated
@@ -1,4 +1,5 @@
# External dependencies
.idea # webstorm/intelliJ ide's etc
Copy link
Member

Choose a reason for hiding this comment

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

Replace the external dep with ide folders

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, sorted 👍

@jsf-clabot
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@@ -31,7 +31,6 @@ In case you are suggesting a new feature, we will match your idea with our curre
*Note: Node 6 or greater would be better for "best results".*
* Fork the **webpack-cli** repo at [https://github.com/webpack/webpack-cli](https://github.com/webpack/webpack-cli).
* `git clone <your-clone-url> && cd webpack-cli`
* Install the commit validator: `npm run install-commit-validator`

### Setup with npm
* Install the dependencies: `npm install`
Copy link
Member

Choose a reason for hiding this comment

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

add npm prepare after the install statement, same for yarn

Copy link
Contributor Author

@peter-mouland peter-mouland Dec 20, 2017

Choose a reason for hiding this comment

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

ah - seems like prepare is a lifecycle hook for both yarn and npm I wasn't aware of. which means it is auto-run after install.

yarnpkg/yarn#3004
https://iamakulov.com/notes/npm-4-prepublish/

Copy link
Member

Choose a reason for hiding this comment

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

gotcha! I tend to do it after installing based on habit 😆

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.

LGTM :shipit: thanks @peter-mouland! Do you want any other issues to work with?

@evenstensberg evenstensberg merged commit 8269461 into webpack:v2 Dec 20, 2017
@evenstensberg evenstensberg mentioned this pull request Dec 20, 2017
@peter-mouland
Copy link
Contributor Author

@ev1stensberg feel free to let me know anything else that would help you out. I'll see what I can do that doesn't clash with whatever you're already working on.

@peter-mouland peter-mouland deleted the v2-docs/remove-commit-validator branch December 20, 2017 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants