Skip to content

feat(ci): Add conventional commit ci check#54

Merged
lukeroantreeONS merged 3 commits intomainfrom
53-add-yaml-to-github-actions-for-conventional-commits
Oct 27, 2025
Merged

feat(ci): Add conventional commit ci check#54
lukeroantreeONS merged 3 commits intomainfrom
53-add-yaml-to-github-actions-for-conventional-commits

Conversation

@Tom-Owen-ONS
Copy link
Collaborator

@Tom-Owen-ONS Tom-Owen-ONS commented Oct 2, 2025

📌 Pull Request Template

Please complete all sections

✨ Summary

This PR adds conventional-commit.yml to the github workflows. It checks to see if PR titles are written in a 'conventional commit' style, which will help to inform the version bump that should be implemented in the following release (feat!: major, feat: minor, fix: patch).

This is mainly useful for if we wish to implement an automated release process, for example release-please, which bumps SemVer numbers based on the prefixes that appear in the commit history. It's also worth noting that such tools heavily favour squash merging as a strategy.

📜 Changes Introduced

  • Added a conventional commit validation GitHub action which checks PR titles follow conventional commit prefix guidance.

  • Feature implementation (feat:) / bug fix (fix:) / refactoring (chore:) / documentation (docs:) / testing (test:)

  • Updates to tests and/or documentation

  • Terraform changes (if applicable)

✅ Checklist

Please confirm you've completed these checks before requesting a review.

  • Code passes linting with Ruff
  • Security checks pass using Bandit
  • API and Unit tests are written and pass using pytest
  • Terraform files (if applicable) follow best practices and have been validated (terraform fmt & terraform validate)
  • DocStrings follow Google-style and are added as per Pylint recommendations
  • Documentation has been updated if needed

🔍 How to Test

Check that this PR conventional commit CI step passes. Confirm the behaviour is as expected.

@Tom-Owen-ONS Tom-Owen-ONS linked an issue Oct 2, 2025 that may be closed by this pull request
@github-actions github-actions bot added the enhancement New feature or request label Oct 2, 2025
@Tom-Owen-ONS Tom-Owen-ONS linked an issue Oct 2, 2025 that may be closed by this pull request
@matweldon
Copy link
Contributor

Just checking because I'm not familiar with how GH actions work: if we add it to a separate yaml does it spin up a separate machine, and then does it use up more budget than just adding as another job in the same yaml?

@matweldon
Copy link
Contributor

Just asking because this seems like quite a simple check - and is a nice-to-have, not essential. I'd be worried about using up significantly more of DSC's Actions budget for it

@Tom-Owen-ONS
Copy link
Collaborator Author

Tom-Owen-ONS commented Oct 3, 2025

@matweldon you get charged per minute and it doesn't appear to cost any different according to the calculator:

image image

But I can't see the billing breakdown as that appears in account settings so I wouldn't know for sure what the difference would be... I guess the advantage of having them separate is that if you were to change the PR title due to a failed conventional commit check, it wouldn't then re-run the other potentially more expensive checks (I think, anyway!).

@matweldon
Copy link
Contributor

@matweldon you get charged per minute and it doesn't appear to cost any different according to the calculator:

But I can't see the billing breakdown as that appears in account settings so I wouldn't know for sure what the difference would be... I guess the advantage of having them separate is that if you were to change the PR title due to a failed conventional commit check, it wouldn't then re-run the other potentially more expensive checks (I think, anyway!).

Good to know, thanks!

@Tom-Owen-ONS
Copy link
Collaborator Author

@matweldon you get charged per minute and it doesn't appear to cost any different according to the calculator:

But I can't see the billing breakdown as that appears in account settings so I wouldn't know for sure what the difference would be... I guess the advantage of having them separate is that if you were to change the PR title due to a failed conventional commit check, it wouldn't then re-run the other potentially more expensive checks (I think, anyway!).

Good to know, thanks!

Something I didn't consider is whether they do something sneaky like round up to the nearest minute for each workflow run... again it'd be useful to see the billing information to confirm if this is what happens as all it seems to say in the docs is you pay per month for usage.

@lukeroantreeONS
Copy link
Collaborator

lukeroantreeONS commented Oct 3, 2025

Just asking because this seems like quite a simple check - and is a nice-to-have, not essential. I'd be worried about using up significantly more of DSC's Actions budget for it

It's charged per compute-minute, so 2 identical jobs concurrently or sequentially started via the same / different YAML files would cost the same. From memory, it's rounded up to the nearest second. Memory betrayed me; GitHub rounds up to the nearest minute, GitLab rounds to the up to the nearest second.

And classifai_package isn't under the enterprise licence so I think it has it's own quota for CICD compute time. (Note, looks to be 2000min/month, I think this budget is shared with the rest of the datasciencecampus private repos. See docs)

lukeroantreeONS
lukeroantreeONS previously approved these changes Oct 6, 2025
Copy link
Collaborator

@lukeroantreeONS lukeroantreeONS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation matches the description proposed, and it seems like it would be a useful feature for tracking changes between releases.
Happy to approve.

@Tom-Owen-ONS
Copy link
Collaborator Author

Tom-Owen-ONS commented Oct 7, 2025

@lukeroantreeONS @matweldon I've added the conventional commit validation to the main ci.yaml check to avoid multiple jobs being made... I was talking to Luke about this recently and there are a couple of issues here:

  • It appears that GitHub does indeed round up to the nearest minute per job when calculating billing. This is understandably something the community aren't happy about. This means it is more cost effective to have a single workflow if each job is small, as it avoids the exponential increase in cost due to this rounding up effect.
  • The ci job as it stands runs on both push to all branches and on all PRs. I believe there isn't a need to run on both because, as you can see, it is running the whole job twice on a PR. Generally you'd have it run on pushes to specific branches (in our case main) and on PR. That way it would run on all PRs and pushes to active PRs, and when these PRs have been merged to main. If we need it to run on all pushes, then I'd suggest removing the 'on: pull_request:' action trigger as it'll run on all pushes anyway and it would avoid this duplicate trigger. Luke had a concern that this would cause it to not trigger on browser edits, but I've just tested it out with a small commit and the jobs still triggered.

What do we think?

@Tom-Owen-ONS Tom-Owen-ONS force-pushed the 53-add-yaml-to-github-actions-for-conventional-commits branch from 4468e2b to 1f85b2c Compare October 7, 2025 16:11
@lukeroantreeONS
Copy link
Collaborator

lukeroantreeONS commented Oct 8, 2025

@lukeroantreeONS @matweldon I've added the conventional commit validation to the main ci.yaml check to avoid multiple jobs being made... I was talking to Luke about this recently and there are a couple of issues here:

  • It appears that GitHub does indeed round up to the nearest minute per job when calculating billing. This is understandably something the community aren't happy about. This means it is more cost effective to have a single workflow if each job is small, as it avoids the exponential increase in cost due to this rounding up effect.
  • The ci job as it stands runs on both push to all branches and on all PRs. I believe there isn't a need to run on both because, as you can see, it is running the whole job twice on a PR. Generally you'd have it run on pushes to specific branches (in our case main) and on PR. That way it would run on all PRs and pushes to active PRs, and when these PRs have been merged to main. If we need it to run on all pushes, then I'd suggest removing the 'on: pull_request:' action trigger as it'll run on all pushes anyway and it would avoid this duplicate trigger. Luke had a concern that this would cause it to not trigger on browser edits, but I've just tested it out with a small commit and the jobs still triggered.

What do we think?

My only holdout on removing the pull-request trigger was that I wanted to confirm the push trigger activates correctly for GitHub web GUI pushes; I'll try testing now by adding a comment to the tidy-up PR. If that goes as expected, then I'm happy to drop the pull-request trigger.

UPDATE: Yeah, that worked as expected (triggered the CICD pipeline), so I'm happy for us to remove the 'on pull request' actions

@Tom-Owen-ONS Tom-Owen-ONS force-pushed the 53-add-yaml-to-github-actions-for-conventional-commits branch from 1f85b2c to cc6b2db Compare October 27, 2025 09:48
@lukeroantreeONS lukeroantreeONS self-requested a review October 27, 2025 10:02
@lukeroantreeONS lukeroantreeONS merged commit 14cc81a into main Oct 27, 2025
3 checks passed
@lukeroantreeONS lukeroantreeONS deleted the 53-add-yaml-to-github-actions-for-conventional-commits branch October 27, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add yaml to Github actions for Conventional commits

3 participants