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/fix commitlint #592

Merged
merged 2 commits into from
Sep 21, 2018
Merged

Feature/fix commitlint #592

merged 2 commits into from
Sep 21, 2018

Conversation

ematipico
Copy link
Contributor

What kind of change does this PR introduce?

Fixed commitlint

Did you add tests for your changes?
No

If relevant, did you update the documentation?
No

Summary
Commitlint was not working properly. It now works perfectly while trying to commit a messge.

Does this PR introduce a breaking change?
No

Other information

@ematipico
Copy link
Contributor Author

ematipico commented Sep 16, 2018

I am going to fix the CI in this PR

EDIT: no idea why on travis it is trying to lint all the commit messages

"type-empty": [2, "never"]
// The scope-enum : defined in the cz-config
// The 'type-enum': defined in the cz-config
// turn it on once CLI will be inside its own package, so the scope will be the name of the pacakged
Copy link
Member

Choose a reason for hiding this comment

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

what do you mean with cli in it's own package?

Copy link
Contributor Author

@ematipico ematipico Sep 16, 2018

Choose a reason for hiding this comment

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

The plungin commitlint for lerna forces to have a commit message where inside the parentheses you have to write down the name of the package. Please read what that package does.

It wasn't working before because the commitlint was half broken.
For now I turned off the rule meanwhile we decide what to do

Copy link
Member

Choose a reason for hiding this comment

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

Okay thanks for info! 💙

@@ -38,6 +38,7 @@
"lint:codeOnly": "eslint \"{bin}/**/!(__testfixtures__)/*.js\" \"{bin}/**.js\"",
"lint": "eslint \"./bin/*.js\" \"./test/**/*.js\" \"packages/**/!(node_modules)/*.test.js\"",
"precommit": "lint-staged",
"commitmsg": "commitlint -E GIT_PARAMS",
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep this simply as commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@webpack-bot
Copy link

@ematipico 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.

@ematipico ematipico merged commit 1e0fa65 into webpack:master Sep 21, 2018
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