Closed gomex closed 4 years ago
Seems to be the last missing piece.
Mmh but then there is nothing the check anymore, so imho you can disable the pull_request at all. I already thinking about to introduce a check for all commits of a PR but before this it does not make much sense imho...
To explain how I use PRs primary: I'm most of the time do a squash merge so the PR title and body are used in the final commit. If you like to do simple merges I think you do not need a setup for the pull_request at all. But I'm also not sure if all commits gets properly checked by the push event. Please describe how you exactly work with PRs so I can find a solution for your workflows...
Without pull_request trigger PR from fork won't trigger the github action. We need to trigger the commit_message_checker to validate the commit from external collaborators.
@gilbertsoft, not always we squash before merge. If all commits are simply logically atomic, it's just a matter of clicking on "Rebase and Merge". So, in this case, I think, commit checker should check all commits of a PR, not the PR title and description.
So we agree all to have a new feature to also check the commits of PR and not just the title and body of it. I'm thinking about a new option for the pull_request event to enable/disable the checks for title, body and commits. This will also mean I will remove the previous provided option and replace it by a more common option which handles all of these three cases.
I think this option should be more flexible to be able to not verify the title and body of the PR, but only the commits related to it. Maybe something like the table bellow:
option | PR Title | PR Body | PR Commits |
---|---|---|---|
1 | 0 | 0 | 1 |
2 | 0 | 1 | 0 |
3 | 0 | 1 | 1 |
4 | 1 | 0 | 0 |
5 | 1 | 0 | 1 |
6 | 1 | 1 | 0 |
7 | 1 | 1 | 1 |
For the example I gave it's enough to only verify commits (option 1). Maybe if each attribute (PR Title, PR body and PR Commits) had boolean variable, this could be easier.
Yes; for me a true
/ false
being true
the default seems to be more user friendly.
I tested this PR #12 and it is working, but I noticed that the PR's title can trigger the rule, is it possible to add an option to remove it too?
https://github.com/gomex/totalcross/runs/1005076797