-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add commitlint #29
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
Conversation
|
Please check this out for PR title as well: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move file to configs folder as done here:
https://github.com/MapColonies/shared-workflows/pull/28/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not finished 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the early review 😋 I will wait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit config should sit on the repo itself like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you removed the workflow on this commit - a5434cb.
This totally changes the PR meaning from adding the workflow to simply adding commitlint.
Why not just add the workflows and enforce them for every PR here? Because I don't want us to add files like package.lock to this repo, it doesn't really belong here IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought so too, but it is meaningless to enforce them in a workflow. That way, the commits are already pushed and the checks will just fail..
I want to prevent the wrong commit message from the beginning. Can you think of an alternative besides husky?
And about the pr-title workflow, it is not belong here so I opened a new PR #30
|
Won't do. As discussed, it is not good to put typescript in workflows repo |
It's important to note that we said this isn't good in the current state of the repo. We still need to add the workflows for this. |
Adds to the repo commitlint so the commits will fit our standards.
In addition, I added workflow that validates that the PR title also fits the standards