ci: drop external action for checking PR title format#2388
ci: drop external action for checking PR title format#2388james-garner-canonical wants to merge 3 commits intocanonical:mainfrom
Conversation
james-garner-canonical
changed the title
james-garner-canonical
changed the title
james-garner-canonical
changed the title
| test | ||
| disallowScopes: ".*" | ||
| persist-credentials: false | ||
| - run: python3 .github/check-conventional-pr-title.py |
There was a problem hiding this comment.
WDYT about attempting a super-compact inline Python block instead?
There was a problem hiding this comment.
I don't love the idea, I think that will be harder to maintain in future
|
|
||
| on: | ||
| pull_request_target: # zizmor: ignore[dangerous-triggers] Doesn't touch code - pull_request results in cancellation errors. | ||
| pull_request: |
There was a problem hiding this comment.
I think we'd be back to cancellation errors, unless the concurrency group is removed or changed.
Maybe it's cleaner to the remove concurrency seeting, given how simple the job is.
There was a problem hiding this comment.
I believe the cancelation errors are only due to there being two workflows running right now, the pull_request trigger one from this branch, and the pull_request_target one from main.
There was a problem hiding this comment.
I doubt that. We used to have on: pull_request originally, and had those cancellation errors. I think it's because pushing the branch and creating a PR triggers both "pr created" and "code pushed" events. TBH I never found the root cause.
There was a problem hiding this comment.
I'm certain that the cancellations we see in this PR are because both the pull_request trigger from this branch, and the pull_request_target trigger from main are running here, and they define the same concurrency group.
I know those aren't the errors you're talking about, just to be clear.
I suggest we merge this PR and keep an eye out for cancellation errors thereafter. If we see the issue you remember, then let's remove the concurrency. I believe the idea with concurrency for this job is to avoid multiple quick PR edits resulting in spurious failures if an intermediate edit made a mistake.
This PR drops the use of an external, third-party action for checking PR title format, replacing it with a small, locally-defined Python script. This has the added security benefit of letting us run this check in the safer `pull_request` context, rather than on `pull_request_target`, which the action apparently required for creating a [commit status](https://docs.github.com/en/rest/commits/statuses?apiVersion=2026-03-10). I'm proposing the same approach for `operator` in canonical/operator#2388
2 participants
This PR drops the use of an external, third-party action for checking PR title format, replacing it with a small, locally-defined Python script. This has the added security benefit of letting us run this check in the safer
pull_requestcontext, rather than onpull_request_target, which the action apparently required for creating a commit status.I'm proposing the same approach for
charmlibsin canonical/charmlibs#380The script in this repo disallows the use of conventional commit scopes, and uses the same set of commits that Ops uses.
In addition to running sanity tests locally with the environment variable set to different values, I've tested it in our CI by editing this PRs title several times.
Due to the workflow's concurrency definition, you'll notice that the
pull_requestversion andpull_request_targetversion race and one gets cancelled. This will be resolved when this PR is merged and only one check is defined.