karma-runner / karma-sauce-launcher

A Karma plugin. Launch any browser on SauceLabs!
MIT License
199 stars 103 forks source link

chore(lint): add commitlint config and dependencies #177

Closed rcebulko closed 4 years ago

rcebulko commented 4 years ago

Reference-commits: https://github.com/karma-runner/karma-jasmine/commit/b2c3cb49aad472111ad13a7dbbef6e39fd488013 and https://github.com/karma-runner/karma-jasmine/commit/8189c8e8e1fc80dbb74f8f7f356ab05ae8c69f60

Addresses #176

wswebcreation commented 4 years ago

Hi @rcebulko

Thanks for the PR, should we also have a docs update where we explain how to use it, or is there a reference link?

rcebulko commented 4 years ago

Hi @rcebulko

Thanks for the PR, should we also have a docs update where we explain how to use it, or is there a reference link?

I'd say https://github.com/conventional-changelog/commitlint would be a sufficient reference link. I didn't see any explicit docs about it in https://github.com/karma-runner/karma-jasmine, so I didn't think it was necessary here. It adds a precommit hook, and when it fails it provides assistive links and an explanation of the failure. Do you think this would be sufficient?

wswebcreation commented 4 years ago

Hi @rcebulko Thanks for the PR, should we also have a docs update where we explain how to use it, or is there a reference link?

I'd say https://github.com/conventional-changelog/commitlint would be a sufficient reference link. I didn't see any explicit docs about it in https://github.com/karma-runner/karma-jasmine, so I didn't think it was necessary here. It adds a precommit hook, and when it fails it provides assistive links and an explanation of the failure. Do you think this wolud be sufficient?

I would prefer something in the readme / contributing docs say something like

We use conventual-changelog to verify the commits. To see how it works please check....

Just as a simple explanation to make it easier to contribute, hope you understand it.

rcebulko commented 4 years ago

It seems like husky is incompatible with node <10, so it fails the node 8 Travis test. I'm not sure how karma-jasmine gets around this. It uses npm and not yarn, so that could be the key difference

rcebulko commented 4 years ago

We use conventual-changelog to verify the commits. To see how it works please check

Added some info and links to CONTRIBUTING.md, let me know what you think

wswebcreation commented 4 years ago

Looks great, thanks

rcebulko commented 4 years ago

[Do not submit] resolving Travis/Yarn issues

rcebulko commented 4 years ago

husky is technically only compatible with Node >=10, so that breaks yarn install in the Travis setup. This PR adds --ignore-engines to the .travis.yml installation script. It also removes the extra yarn installation script, since Travis does this already (which is why yarn --frozen-lockfile can be seen in the logs before the yarn invocation).

Travis is still failing trying to connect to Sauce Labs because of missing tokens. That is unrelated to this change.

@wswebcreation PTAL

rcebulko commented 4 years ago

@wswebcreation Moved this to https://github.com/karma-runner/karma-sauce-launcher/pull/181 and it passes all tests

rcebulko commented 4 years ago

Closing (merged in #181 )