ci: ensure title matches Conventional Commits spec #187
Conversation
deepyaman
changed the title
|
👀How Is this gonna work does it reject invalid pr immediately? |
|
Ah I guess it will be just another check, so it won't prevent people from opening invalid PR but just blocking the merge of it. What is a valid PR title here? |
Yes, correct. I would follow the Conventional Commits specification, with scope as one of airflow, datasets, docker, etc. (Also, sorry, it's still WIP; I just marked it as ready to trigger CI!) |
deepyaman
changed the title
deepyaman
changed the title
deepyaman
changed the title
deepyaman
changed the title
There was a problem hiding this comment.
I checked to see if it's possible to have a PR title in the template, it doesn't seem to be possible. The rules are simple enough so I think it's ok.
isaacs/github#1252
Question:
- What about commit doesn't fall into these scope? Like CI, i.e. is this PR itself regard as a valid commit?
p.s. I think we should have a CONTRIBUTOR.md here.
Yep! I didn't require a scope, because (like you pointed out) not all PRs will apply to just one plugin.
Totally agree, and I'd even be happy to start one off with just the guidelines above + some stuff copied over from the Kedro/Kedro-Viz CONTRIBUTING.md? I'm not 100% sure what it should look like, so maybe it's better I leave it out of this PR. On a separate note, it may also make sense to not maintain separate CONTRIBUTING.md for each repo (e.g. with Code of Conduct, etc.). |
How are we dealing with it? I am seeing the checking failed for this PR |
Where? I'm seeing it passed. 😅 I did modify it to "ci(invalid-scope): ensure title matches Conventional Commits spec" at some point before, so maybe the failure is associated with that? |
There was a problem hiding this comment.
+1 on adding a CONTRIBUTING.md here but also might be worth adding a note in the PR template itself so people can see it when they open the PR - like we did for kedro repository when we moved kedro-datasets
* Create validate-pr-title.yaml * ci: add `ready_for_review` to the PR type triggers * Update validate-pr-title.yaml * revert: drop the `ready_for_review` type from list * ci: restrict the set of scopes to the plugin names Signed-off-by: Danny Farah <danny_farah@mckinsey.com>
Description
Challenges
[...]
[kedro-docker]on PR/issues (nonstandard solution)Proposal
[...]
commitlint-- rough estimate: trivial, <0.5 daysOriginally posted by @deepyaman in kedro-org/kedro#2333 (comment)
Development notes
Tested manually. Also, have used the Action on other projects previously. The configuration is pretty much that documented in the Action's repo.
Can somebody with rights ensure that the squash-merge strategy IS NOT set to use the commit message if there's only one commit? See https://github.com/amannn/action-semantic-pull-request#installation.
Is there (going to be) a CONTRIBUTING.md for this repo? If so, we can include the following:
Pull request title conventions
The Kedro-Plugins repository requires that you squash and merge your pull request commits, and, in most cases, the merge message for a squash merge then defaults to the pull request title.
Your pull request title must match the Conventional Commits spec. Also see the list of supported commit types and their descriptions. Commit scope can be one of the plugin names.
Examples
Commit message with no body
ci: port lint, unit test, and e2e tests to ActionsCommit message with scope
build(datasets): add the missing `snowflake` extraChecklist
RELEASE.mdfile