loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.96k stars 1.07k forks source link

ci: Commit Linter needs to allow multiple packages listed in scope #581

Closed virkt25 closed 6 years ago

virkt25 commented 7 years ago

Overview

A key benefit of lerna is the ability of being able to change multiple packages if needed in one PR to make fixes and feature implementation easier.

Conventional Commit message guidelines require the header format to be <type>(<scope>): <description> where scope is the affected package name. Travis CI enforces this convention using commitlint.

Depending on the outcome of https://github.com/marionebl/commitlint/issues/75 we need to create the new scope-types rule (if / when) there is a plugin API available for commitlint OR We need to implement our own solution that can leverage conventional-commits-parser with a custom headerPattern (adding , to the default pattern). Then split it and check against available packages (check commitlint's lerna config for getting scope-enum automatically).

Acceptance Criteria

marionebl commented 7 years ago

Relevant issue with conventional-commits-parser: https://github.com/conventional-changelog/conventional-changelog/issues/232

stale[bot] commented 6 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

bajtos commented 6 years ago

IMO, this is a "nice to have" feature. Commits modifying multiple packages are omitting the scope now and I don't remember any issues caused by the missing scope.

I am proposing to close this issue and focus on more important tasks.

@virkt25 thoughts?

virkt25 commented 6 years ago

I agree, plus this change is something we would pick up from conventional-commits if it's implemented there. Closing for now.