chore: add lefthook, PR workflows, and CI standardization#1
Conversation
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
10a5499 to
a153cfd
Compare
✅ StructLint Validation
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Core Changes
- Introduces
lefthookfor pre-commit (gofmt, govet) and commit-msg (conventional commit validation) hooks. - Adds GitHub Actions workflows for PR title validation, automated code review (
reviewforge), and project structure validation (structlint). - Standardizes the
test.ymlworkflow to run only onmainbranch pushes and pull requests targetingmain.
Concerns
⚠️ The lefthook.yml commit message validation regex does not fully align with the Conventional Commits specification, specifically missing support for the breaking change indicator (!). This will cause valid commit messages to fail locally.
⚠️ The test.yml workflow's scope has been narrowed from "**" to main for push events and from all PRs to only PRs targeting main. This is a breaking change in terms of test coverage for other development branches.
Verdict
Request Changes due to the identified logical error in the lefthook.yml regex and the significant change in test workflow coverage.
Code review performed by GEMINI - gemini-2.5-flash.
.github/workflows/test.yml
Outdated
| on: | ||
| push: | ||
| branches: ["**"] | ||
| branches: |
There was a problem hiding this comment.
[CRITICAL] The push event's branches filter has been changed from "**" to main. This means pushes to any branch other than main will no longer trigger the test workflow. This is a breaking change in workflow coverage and might not be intended if feature branches are expected to run tests on push.
There was a problem hiding this comment.
Fixed — push now triggers on all branches, pull_request on main only.
| branches: ["**"] | ||
| branches: | ||
| - main | ||
| pull_request: |
There was a problem hiding this comment.
[CRITICAL] The pull_request event's branches filter has been added to main. This means pull requests targeting branches other than main will no longer trigger the test workflow. This is a breaking change in workflow coverage and might not be intended if feature branches are expected to run tests on PR.
There was a problem hiding this comment.
Fixed — push now triggers on all branches, pull_request on main only.
| run: | | ||
| msg=$(cat {1}) | ||
| if ! echo "$msg" | grep -qE "^(feat|fix|docs|style|refactor|test|chore|build|ci|perf|revert)(\(.+\))?: .+"; then | ||
| echo "Error: Commit message must follow conventional commits format" |
There was a problem hiding this comment.
[CRITICAL] The Conventional Commits regex ^(feat|fix|docs|style|refactor|test|chore|build|ci|perf|revert)(\(.+\))?: .+$ is missing the optional breaking change indicator ! after the scope. A commit message like feat!: add breaking change would be valid Conventional Commit but would fail this local hook. This regex should be consistent with the one used in .github/workflows/pr-title.yml.
There was a problem hiding this comment.
Fixed — regex now includes (!)? for breaking change indicator.
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
a153cfd to
16da5bc
Compare
There was a problem hiding this comment.
Core Changes
- Introduced new GitHub Actions workflows for Pull Request validation, including Conventional Commit title checks, AI-powered code review (
reviewforge), and structural linting (structlint). - Standardized the
test.ymlworkflow to run onmainbranch pushes and PRs targetingmain, and updated Go setup. - Added
lefthookconfiguration for pre-commit (gofmt, govet) and commit-msg (conventional commit linting) hooks. - Defined project structure and file naming conventions using
.structlint.yaml.
Concerns
⚠️ Thecommit-msghook inlefthook.ymluses a Conventional Commits regex that does not account for the optional breaking change marker (!), leading to an inconsistency with the PR title validation workflow.
Verdict
Request Changes: The inconsistency in Conventional Commit regex between lefthook.yml and pr.yml is a logical error that will cause valid commit messages to be rejected by the local hook, even if they would pass the PR title check. This needs to be fixed.
Code review performed by GEMINI - gemini-2.5-flash.
| run: | | ||
| msg=$(cat {1}) | ||
| if ! echo "$msg" | grep -qE "^(feat|fix|docs|style|refactor|test|chore|build|ci|perf|revert)(\(.+\))?: .+"; then | ||
| echo "Error: Commit message must follow conventional commits format" |
There was a problem hiding this comment.
[CRITICAL] The regex ^(feat|fix|docs|style|refactor|test|chore|build|ci|perf|revert)(\(.+\))?: .+$ used here for commit message validation does not include the optional breaking change marker ! (e.g., feat!: description). This is inconsistent with the PR title validation in .github/workflows/pr.yml (line 22) which correctly includes (!)?:. Please update this regex to match the more complete one, allowing for breaking changes in commit messages.
There was a problem hiding this comment.
Fixed — regex now includes (!)? for breaking change indicator.
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
16da5bc to
a84a1b7
Compare
There was a problem hiding this comment.
Core Changes
- Introduced new GitHub Actions workflows for PR validation, including Conventional Commits title checks, AI-powered code review (ReviewForge), and project structure validation (StructLint).
- Added
lefthookconfiguration for client-side Git hooks, enforcinggofmt,govet, and Conventional Commits for commit messages. - Standardized the
test.ymlworkflow by updating step names and modifying the trigger to run only on pull requests targeting themainbranch.
Concerns
test.yml to trigger only on pull_request events targeting main significantly reduces test coverage compared to the previous configuration which ran on all push events to all branches. This means pushes to feature branches or direct pushes to main will no longer trigger tests, potentially degrading the existing functionality of the CI pipeline.
Verdict
Request Changes: The change in the test.yml workflow's trigger condition is a significant functional alteration that reduces test coverage. This needs to be explicitly addressed to ensure it aligns with the project's testing strategy and does not inadvertently introduce regressions by skipping tests on certain code changes.
Code review performed by GEMINI - gemini-2.5-flash.
| on: | ||
| push: | ||
| branches: ["**"] | ||
| pull_request: |
There was a problem hiding this comment.
[WARNING] The change from push: branches: ["**"] to pull_request: branches: [main] means that tests will no longer run on pushes to feature branches or direct pushes to main. This significantly reduces the scope of CI testing. If the intention is to only run tests on PRs targeting main, this is a functional change that should be explicitly acknowledged. Otherwise, it could lead to a degradation of test coverage.
There was a problem hiding this comment.
Fixed — push now triggers on all branches, pull_request on main only.
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
a84a1b7 to
d52fc98
Compare
There was a problem hiding this comment.
Core Changes
- Introduced
lefthook.ymlfor pre-commit and commit-msg hooks, enforcinggofmt,govet, and Conventional Commits. - Added
.github/workflows/pr.ymlto validate PR titles against Conventional Commits, runreviewforgefor AI code review, andstructlintfor project structure validation. - Created
.github/workflows/releaseforge.ymlas a reusable workflow forReleaseForgeoperations (bump, generate). - Added
action.ymlas a composite GitHub Action forReleaseForge, handling binary download and command execution. - Introduced
.structlint.yamlto define and enforce project directory structure and file naming conventions.
Concerns
test.yml workflow has been modified to remove the push trigger for all branches and restrict pull_request triggers to only the main branch. This significantly reduces test coverage, as tests will no longer run on pushes to any branch or on PRs targeting non-main branches. This change is not mentioned in the PR description and could lead to undetected issues in feature branches or direct pushes.
Verdict
Request Changes: The change to test.yml is a critical regression in CI coverage and needs to be addressed or explicitly justified and documented. If this change was unintentional, it should be reverted. If intentional, the rationale should be provided, and alternative testing strategies for other branches should be considered.
Code review performed by GEMINI - gemini-2.5-flash.
| @@ -2,18 +2,20 @@ name: Test | |||
|
|
|||
| on: | |||
| push: | |||
There was a problem hiding this comment.
[CRITICAL] The on: push: trigger has been removed, and pull_request is now restricted to branches: - main. This means tests will no longer run on pushes to any branch or on pull requests targeting branches other than main. This is a significant reduction in CI coverage and could lead to issues being introduced without immediate detection. Please clarify if this change is intentional. If not, the original branches: ["**"] for push and an unrestricted pull_request trigger should be restored, or a more comprehensive testing strategy for all branches should be implemented.
There was a problem hiding this comment.
Fixed — push now triggers on all branches, pull_request on main only.
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
d52fc98 to
ac8429f
Compare
There was a problem hiding this comment.
Core Changes
- Introduces new GitHub Actions workflows for PR validation, including Conventional Commits title checks, AI-powered code review, and project structure validation with
structlint. - Enhances the release workflow to support automatic version bumping based on Conventional Commits, making the
taginput optional. - Adds
lefthookconfiguration for pre-commit hooks (gofmt, govet) and commit-msg hooks (Conventional Commits validation). - Standardizes the
test.ymlworkflow trigger to run only on pull requests targeting themainbranch.
Concerns
test.yml workflow's trigger has been changed from push on all branches to pull_request on the main branch only. This significantly reduces test coverage as pushes to feature branches or other non-main branches will no longer trigger tests. This could lead to issues being introduced on branches that are not directly PR'd to main.
Verdict
Request Changes: The change in the test.yml workflow's trigger is a breaking change in CI coverage that needs to be confirmed as intentional or adjusted to ensure adequate testing across all relevant branches.
Code review performed by GEMINI - gemini-2.5-flash.
| @@ -2,18 +2,20 @@ name: Test | |||
|
|
|||
| on: | |||
| push: | |||
There was a problem hiding this comment.
[WARNING] The on: push trigger for all branches has been removed, and on: pull_request is now limited to the main branch. This means pushes to feature branches or other non-main branches will no longer trigger tests. Please confirm this reduction in test coverage for non-main branches is intentional. If testing is still required for pushes to other branches, the push trigger should be re-added or adjusted.
There was a problem hiding this comment.
Fixed — push now triggers on all branches, pull_request on main only.
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
ac8429f to
fa262e7
Compare
There was a problem hiding this comment.
Core Changes
- Introduced new GitHub Actions workflows for Pull Request (PR) validation, including conventional commit title checks, AI-powered code review (
reviewforge), and structural linting (structlint). - Enhanced the release workflow (
release.yml) to automate version bumping and AI-powered release notes generation using thereleaseforgetool, making thetaginput optional for auto-detection. - Added
lefthook.ymlfor pre-commit and commit-msg hooks, enforcinggofmt,govet, and conventional commit message format locally.
Concerns
test.yml workflow has been modified to only trigger tests on pull_request events targeting the main branch, and on push events to any branch. This means PRs targeting branches other than main will no longer automatically run tests, which could lead to reduced test coverage for feature or release branches.
Verdict
Request Changes: The change in test.yml significantly reduces test coverage for pull requests not targeting the main branch. This is a functional change that could lead to regressions or issues being merged without proper testing. It should be addressed to ensure all PRs are adequately tested.
Code review performed by GEMINI - gemini-2.5-flash.
| push: | ||
| branches: ["**"] | ||
| pull_request: | ||
| branches: |
There was a problem hiding this comment.
[WARNING] The pull_request event is now configured to only trigger for PRs targeting the main branch. This means that pull requests opened against other branches (e.g., feature branches, release branches) will no longer automatically run tests. This significantly reduces test coverage and could allow issues to be introduced into non-main branches without being caught by CI. Consider removing the branches: - main filter to ensure all pull requests run tests, or explicitly define the desired branches if this reduction in coverage is intentional.
There was a problem hiding this comment.
Fixed — push now triggers on all branches, pull_request on main only.
machado144
force-pushed
the
chore/standardize-ci-tooling
branch
from
fa262e7 to
6066328
Compare
There was a problem hiding this comment.
Core Changes
- Introduced
lefthook.ymlfor pre-commit and commit-msg hooks, enforcinggofmt,govet, and Conventional Commits. - Added a new GitHub Actions workflow (
pr.yml) to validate PR titles against Conventional Commits, runreviewforgefor AI code review, andstructlintfor project structure validation. - Enhanced the
release.ymlworkflow to support automatic version bumping and AI-powered release note generation usingreleaseforge, making thetaginput optional. - Created a reusable GitHub Action (
action.yml) forreleaseforgeto simplify its integration into other workflows.
Verdict
Approve: The changes introduce robust CI/CD practices, automate versioning and release notes, and enforce code quality standards. All new configurations and workflow logic appear sound and correctly implemented.
Code review performed by GEMINI - gemini-2.5-flash.
## Summary - Add lefthook.yml with gofmt, govet, and conventional commit hooks - Add PR review workflow (reviewforge) for automated code review - Add structlint validation workflow for PRs - Add PR title validation for conventional commits (squash merge support) - Standardize test.yml with setup-go@v5 and caching ## Test plan - [ ] Verify `go test ./...` passes - [ ] Check workflows render correctly in GitHub Actions tab
1 participant
Summary
Test plan
go test ./...passes