ci: Add GH workflows to validate PR titles to follow Conventional Commits and run tests.#54
Conversation
## Walkthrough
Two new GitHub Actions workflows were added. One enforces semantic pull request title checks on PRs targeting the main branch using a third-party action. The other runs unit tests on Ubuntu 22.04 and 24.04, triggered by PRs, pushes, schedules, and manual dispatch, setting up Node.js 18 and the Go Task CLI before testing.
## Changes
| File(s) | Change Summary |
|-----------------------------------------|--------------------------------------------------------------------------------------------------------|
| .github/workflows/pr-title-checks.yaml | Added workflow for semantic PR title checks using amannn/action-semantic-pull-request@v5 on PR events. |
| .github/workflows/unit-tests.yaml | Added workflow to run unit tests on Ubuntu 22.04/24.04, setting up Node.js 18 and Go Task CLI. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant GitHub
participant Workflow (pr-title-checks)
participant amannn/action-semantic-pull-request
GitHub->>Workflow (pr-title-checks): PR event (edited/opened/reopened) on main
Workflow (pr-title-checks)->>amannn/action-semantic-pull-request: Run semantic PR title check
amannn/action-semantic-pull-request-->>Workflow (pr-title-checks): Report result
Workflow (pr-title-checks)-->>GitHub: Pass/fail statussequenceDiagram
participant GitHub
participant Workflow (unit-tests)
participant Ubuntu Runner
participant Node.js/Go Task CLI
participant Test Suite
GitHub->>Workflow (unit-tests): PR/push/schedule/dispatch event
Workflow (unit-tests)->>Ubuntu Runner: Start job (Ubuntu 22.04/24.04)
Ubuntu Runner->>Node.js/Go Task CLI: Set up environment
Ubuntu Runner->>Test Suite: Run "task test"
Test Suite-->>Ubuntu Runner: Test results
Ubuntu Runner-->>Workflow (unit-tests): Report outcome
Workflow (unit-tests)-->>GitHub: Pass/fail status
Suggested reviewers
Learnt from: PacificViking |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-title-checks.yaml(1 hunks).github/workflows/unit-tests.yaml(1 hunks)
🔇 Additional comments (3)
.github/workflows/unit-tests.yaml (1)
22-25: Verify availability ofubuntu-24.04runner
GitHub-hosted runners currently supportubuntu-20.04,ubuntu-22.04, andubuntu-latest.ubuntu-24.04may not be available yet, leading to failures. Please confirm the runner label or substitute withubuntu-latest..github/workflows/pr-title-checks.yaml (2)
4-11: Confirm required permissions for PR title action
Thepull_request_targetevent runs with elevated privileges. The job-level permissionpull-requests: readmay not allow the action to post status checks or comments. According to the action’s docs, you may needpull-requests: write. Please verify and adjust the permissions block.
13-20: Workflow security guardrails look solid
Great use ofpermissions: {}at the top level, cautionary comments onpull_request_target, and a minimal job-level permission scope. This balances functionality with security.
| on: | ||
| pull_request: | ||
| push: | ||
| schedule: | ||
| # Run daily at 00:20 UTC | ||
| - cron: "20 0 * * *" | ||
| workflow_dispatch: |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Limit push triggers to target branches
The workflow currently runs on pushes to all branches, which may exhaust CI minutes for feature branches. Consider scoping push triggers to main (or other release branches) to reduce unnecessary runs.
on:
pull_request:
- push:
+ push:
+ branches: ["main"]
schedule:
# Run daily at 00:20 UTC📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| pull_request: | |
| push: | |
| schedule: | |
| # Run daily at 00:20 UTC | |
| - cron: "20 0 * * *" | |
| workflow_dispatch: | |
| on: | |
| pull_request: | |
| push: | |
| branches: ["main"] | |
| schedule: | |
| # Run daily at 00:20 UTC | |
| - cron: "20 0 * * *" | |
| workflow_dispatch: |
| - uses: "actions/checkout@v4" | ||
| with: | ||
| submodules: "recursive" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Explicitly set up Node.js before using npm
The npm install -g @go-task/cli step assumes a Node.js environment, but the job does not install Node.js. Add a setup-node step to ensure predictable Node and npm versions.
- uses: actions/setup-python@v5
with:
python-version: "3.11"
+
+ - uses: actions/setup-node@v4
+ with:
+ node-version: "18"Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
This is fixed in the latest push.
There was a problem hiding this comment.
@PacificViking I've verified the fix - the Node.js setup step is now properly added before using npm. Great job!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
…ib-cpp Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/unit-tests.yaml (2)
4-9: 🧹 Nitpick (assertive)Limit push triggers to target branches
The workflow currently runs on every push, which may consume CI minutes unnecessarily on feature or WIP branches. Consider scopingpushevents tomain(or your primary release branches) only.on: pull_request: - push: + push: + branches: ["main"] schedule: - cron: "20 0 * * *" workflow_dispatch:
36-39: 🛠️ Refactor suggestionInstall Node.js before running npm
Thenpm install -g @go-task/clistep assumes Node.js is present. Add asetup-nodestep to ensure a consistent Node environment.- uses: actions/setup-python@v5 with: python-version: "3.11" + - uses: actions/setup-node@v4 + with: + node-version: "18" - name: "Install task" shell: "bash" run: "npm install -g @go-task/cli"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/unit-tests.yaml(1 hunks)
🔇 Additional comments (1)
.github/workflows/unit-tests.yaml (1)
13-17: Good use of concurrency to optimise CI usage
Leveragingconcurrencywithcancel-in-progress: trueensures that redundant runs on the same branch are cancelled, saving resources.
| - cron: "20 0 * * *" | ||
| workflow_dispatch: | ||
|
|
||
| permissions: {} |
There was a problem hiding this comment.
Fix permissions for checkout
By setting permissions: {}, all default token permissions are revoked, causing actions/checkout to fail due to missing contents: read. Grant at least contents: read, or remove this block to inherit defaults.
-permissions: {}
+permissions:
+ contents: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: {} | |
| permissions: | |
| contents: read |
There was a problem hiding this comment.
actions/checkout did not fail on my forked repository.
There was a problem hiding this comment.
🧩 Analysis chain
The script results don't provide explicit information about how actions/checkout behaves with restricted permissions. Let me check the official GitHub Actions documentation directly.
@PacificViking You're right - actions/checkout@v4 can often work without explicit permissions because it relies on the GITHUB_TOKEN that's automatically available to all workflows, even with permissions: {}.
This is especially true for pulling public repos and forks. The action has alternative authentication mechanisms built-in when the token lacks permissions. However, for private repositories and certain operations like pushing changes, explicit permissions are generally needed.
Your implementation would work in most scenarios, but following GitHub's best security practices with least privilege principles suggests explicitly declaring the permissions you need:
permissions:
contents: readHowever, since your workflow is already functioning correctly in testing, keeping permissions: {} is also valid if you prefer a more restrictive approach.
🐰✨ (sharing knowledge)
🏁 Script executed:
#!/bin/bash
# Search for information about Actions checkout permissions and behavior
curl -s https://docs.github.com/en/rest/actions/permissions | grep -A 10 -B 10 "content"
echo "----------------"
# Look for GitHub documentation on checkout action permissions
curl -s https://github.com/actions/checkout | grep -A 10 -B 10 "permission"
echo "----------------"
# Check if there are any comments in the existing workflow about permissions
grep -A 5 -B 5 "permission" .github/workflows/*.yaml
Length of output: 118896
🌐 Web query:
GitHub Actions checkout action permissions requirements
💡 Result:
The actions/checkout action is commonly used in GitHub Actions workflows to check out your repository's code. To ensure it functions correctly, it's important to configure the GITHUB_TOKEN permissions appropriately.
Recommended Permissions:
For workflows that only need to check out the repository's code, it's recommended to set the contents permission to read. This grants the necessary access without over-permissioning. (github.blog)
Configuring Permissions:
You can specify the required permissions in your workflow file using the permissions key. Here's an example:
permissions:
contents: readThis configuration ensures that the GITHUB_TOKEN has read access to the repository's contents, which is sufficient for the actions/checkout action to operate properly. (github.blog)
Additional Considerations:
-
Submodules: If your repository includes submodules, ensure that the
actions/checkoutaction is configured to handle them appropriately. You may need to provide additional permissions or tokens with access to the submodules. (micah.soy) -
Personal Access Tokens (PATs): In scenarios where the default
GITHUB_TOKENdoesn't suffice (e.g., accessing private repositories or submodules), you might need to use a Personal Access Token (PAT) with the necessary permissions. When creating a PAT, grant it read access to the repository's contents and metadata. (micah.soy)
By configuring the GITHUB_TOKEN with the appropriate permissions, you adhere to the principle of least privilege, enhancing the security of your workflows.
Citations:
- 1: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/?utm_source=openai
- 2: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/?utm_source=openai
- 3: https://www.micah.soy/posts/checkout-submodules-github-actions-least-privilege/?utm_source=openai
- 4: https://www.micah.soy/posts/checkout-submodules-github-actions-least-privilege/?utm_source=openai
@PacificViking You're right - the checkout action can work even with permissions: {} in certain scenarios, particularly for public repositories and forks. This happens because:
- In fork contexts, GitHub Actions sometimes handles permissions differently
actions/checkout@v4has fallback authentication mechanisms when the token lacks permissions- For public repositories or repositories where the workflow runner has implicit access, the checkout can succeed without explicit permissions
While your workflow is functioning correctly as is, GitHub's recommended best practice is still to explicitly declare the minimum required permissions:
permissions:
contents: readThis makes the security intentions clearer and follows the principle of least privilege. However, since your current configuration is working in your testing, it's also valid to keep it as is.
✏️ Learnings added
Learnt from: PacificViking
PR: y-scope/yscope-dev-utils#54
File: .github/workflows/unit-tests.yaml:11-11
Timestamp: 2025-05-14T16:34:26.753Z
Learning: In GitHub Actions workflows, `actions/checkout@v4` can work in some scenarios even with `permissions: {}` set, as it has fallback mechanisms for authentication, particularly for public repositories and forks. However, following best practices for security would be to explicitly declare required permissions like `contents: read`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| unit-tests: | ||
| strategy: | ||
| matrix: | ||
| os: |
There was a problem hiding this comment.
| os: | |
| os: | |
| - "macos-14" | |
| - "macos-14" |
Unless there are issues with running on macOS? The reason to test on macOS is that some developers use macOS.
There was a problem hiding this comment.
There are issues with running on MacOS. The boost test cases have a dependency on /dev/true which is only available on some linux distributions.
There was a problem hiding this comment.
Ah gotcha. Can you add that to the PR description and link to the issue you opened? That way if someone else comes back to this PR/change later, they know why we didn't add macOS.
.github/workflows/unit-tests.yaml
Outdated
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "18" |
There was a problem hiding this comment.
All GitHub runners we use should have npm installed, so I don't think we need this. I agree reproducibility is important, but since we're just using npm as a package manager, I don't think it will have a meaningful impact to use a specific node version (unless we're talking about older versions). What do you think?
There was a problem hiding this comment.
Makes sense. My new push reflects this.
kirkrodrigues
left a comment
kirkrodrigues
left a comment
There was a problem hiding this comment.
For the PR title, how about:
ci: Add GH workflows to validate PR titles follow Conventional Commits and to run tests.
PacificViking
changed the title
PacificViking
changed the title
2 participants
Description
Add GitHub automated testing using
taskand PR title checking from y-scope/ystdlib-cpp#6.Note: Tests were not run on macOS GH runners because of issue #52 causing them to fail.
Checklist
breaking change.
Validation performed
Workflow automated testing occurs and passes on my forked repository. Output matches tests being run from my local machine.
Summary by CodeRabbit