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

Feature/setup commits validation and contributors readme #3

Merged

Conversation

DanielaValero
Copy link
Contributor

@DanielaValero DanielaValero commented Dec 16, 2016

What this PR do?

  • Adds a contributor documentation, all the sections are taken from webpack and webpack.js.org
  • Proposes using a subset of git-flow as branching model. Basically using prefixes for new features and bugfixes. More info in its own commit: abb5f7a
  • Adds an option to use yarn for the dependencies management
  • Adds the editorcofig a js beautifier settings of webpack
  • Adds a commit message validation
  • Proposes a template for commit messages. Same as the one of angularJS, but without the scope. More info in its own commit: eda7553

Where should the reviewer start?

  • Checking the contributor documentation
  • Checking proposed branching model
  • Checking the proposed commit messages template

How to test manually

For the yarn setup

Just follow the installation instrucctions in the contributing.

For the commit validation

Once you checked out the branch, do:

  • npm i: Will install the new dependency
  • npm run install-commit-validator: Will link to pre-commit hook the cli of the commit validator

Where should the reviewer start?

Checking the contributor documentation

@jsf-clabot
Copy link

jsf-clabot commented Dec 16, 2016

CLA assistant check
All committers have signed the CLA.

@DanielaValero DanielaValero force-pushed the feature/setup-commits-validation-and-contributors-readme branch from 711da16 to b6f8f98 Compare December 16, 2016 19:16
In order to have a consistent style on the
projects of webpack, we are using for the cli the
settings that are used also in webpack.
@DanielaValero DanielaValero force-pushed the feature/setup-commits-validation-and-contributors-readme branch 2 times, most recently from a51e888 to 84ff343 Compare December 16, 2016 20:28
@evenstensberg evenstensberg self-assigned this Dec 16, 2016
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.

Needs change:

  • Proper indenting of the filename
  • Rephrasing the first sentence starting with webpack is insanely...
  • .gitignore follows same style


If you are still having difficulty after looking over your configuration carefully, please post
a question to [StackOverflow with the webpack-cli tag](http://stackoverflow.com/tags/webpack-cli). Questions
that include your webpack.config.js and relevant files, this way you help others to help you.
Copy link
Member

Choose a reason for hiding this comment

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

Use webpack.config.js as this is a filename

Copy link
Contributor Author

@DanielaValero DanielaValero Dec 16, 2016

Choose a reason for hiding this comment

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

Hi, I don't understand quite right what you mean here, would you be more specific about the change ?... Ah, I got it you meant adding the quotes

Copy link
Member

Choose a reason for hiding this comment

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

Just replace webpack.config.js with webpack.config.js 👍


## Documentation

webpack is insanely feature rich and documentation is a time sink. We
Copy link
Member

Choose a reason for hiding this comment

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

Consider rephrasing the first sentence, might be somewhat unclear what you meant

@DanielaValero
Copy link
Contributor Author

Checking the errors of travis

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.

Should follow the rest of the file style

npm-debug.log
yarn-error.log
Copy link
Member

Choose a reason for hiding this comment

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

Should be #yarn-debug log and the log should be placed under there or you change #npm-debug log to npm and yarn-debug log

Yarn provides a good hablding of the dependencies
on top of npm, using yarn has been adopted not
only in the webpack project, but also lots of the
loaders. In order to keep consitency across
projects, adding it here also.
In order to have an automated workflow to style
our code, adding now the beautifier settings of
webpack.
In order to allow others to contribute to the
project, we need to specify the rules and
workflows with which we work. Adding them now in
the right place.
In order to allow reviewers of PRs to get quicker
into contex and estimate review time of a PR,
adding a section to specify a branching model for
the project. Basically it takes the feature and
bugfix features of git-flow.
When submitting a contribution and opening a PR,
the cla bot will automatically be launched. We
need to explain the devs what is this about.
@DanielaValero DanielaValero force-pushed the feature/setup-commits-validation-and-contributors-readme branch from 84ff343 to 9b828c0 Compare December 16, 2016 22:06
In order to have a similar structure in the commit
messages of the contributions, adding a commit
validation hook, that a subset of the structure of
angular.js validation messages.
In order to have a similar strucuture with the
core project, adding now the issue and pr
templates.
@evenstensberg evenstensberg merged commit 2731cc3 into master Dec 16, 2016
@evenstensberg evenstensberg deleted the feature/setup-commits-validation-and-contributors-readme branch December 16, 2016 22:35
@evenstensberg
Copy link
Member

Thx! Nice work @DanielaValero !

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