chore: GitHub Actions #162
Conversation
Updated the deployment workflow to trigger only on pushes to main or master branches, ensuring deployments only occur after code is merged to protected branches.
…present Modified the Go formatting workflow to: - Trigger on every PR commit (opened, reopened, synchronize) - Skip formatting only when PR has 'no-fmt' label - Auto-commit formatting fixes on draft PRs (respects 'no-fmt' label) - Removed dependency on 'style' label for auto-commits This ensures consistent code formatting across all PRs by default, with an opt-out mechanism via the 'no-fmt' label.
Changes: - Remove 'master' branch from all workflow triggers (deploy.yml, tests.yml) - Remove push trigger on main from gofmt.yml to prevent auto-formatting on merged PRs - Clean up gofmt.yml by removing main branch handling logic and PR creation steps All workflows now only target the 'main' branch, and gofmt only runs on pull requests (unless 'no-fmt' label is present).
|
Caution Review failedThe pull request is closed. WalkthroughThis PR updates two GitHub Actions workflows: the gofmt workflow is simplified and its pull_request triggers broadened; the tests workflow removes the master branch from push triggers (now only main). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub Events
participant Gofmt as gofmt Workflow
participant Repo as Repository
Dev->>GH: open/reopen/synchronize PR (or label/unlabel)
GH->>Gofmt: trigger workflow (pull_request)
alt PR not labeled "no-fmt"
Gofmt->>Gofmt: run formatting job
Gofmt->>Repo: Commit Formatting Changes (if any)
else PR labeled "no-fmt"
Gofmt-->>GH: skip formatting job
end
Note over Gofmt,Repo: Removed automated main->branch creation and PR opening on push
sequenceDiagram
autonumber
participant Push as Push to main
participant OldFlow as Old workflow (pre-change)
participant NewFlow as New workflow (post-change)
Push->>OldFlow: create formatting branch -> open PR (automated)
Note right of OldFlow: Automated branch+PR creation existed before
Push->>NewFlow: no automated branch/PR creation
Note right of NewFlow: Simplified — no multi-step automation on push
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Removed draft PR restriction from auto-commit logic, allowing gofmt to automatically apply formatting changes on all pull requests (unless 'no-fmt' label is present). Security: Auto-commits only apply to PRs from the same repository, not forks.
Removed confusing "dry run" step and streamlined the workflow to: 1. Run gofmt which modifies files in place 2. Auto-commit any formatting changes directly to the PR The git-auto-commit-action automatically detects changes made by gofmt and commits them to the PR branch. If no changes are detected, it skips the commit step.
gocanto
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/gofmt.yml (2)
13-15: Consider simplifying the job condition.The condition includes a check for
github.event_name == 'pull_request', which is always true since the workflow only triggers on pull_request events. While this is defensive (helpful if push triggers are added later), it can be simplified to just the label check for current code clarity.If you're confident no push triggers will be added to this workflow, you can simplify to:
gofmt: if: > - github.event_name == 'pull_request' && - !contains(github.event.pull_request.labels.*.name, 'no-fmt') + !contains(github.event.pull_request.labels.*.name, 'no-fmt')Alternatively, keep the current form as a defensive guard against future push trigger additions.
37-40: Redundant no-fmt label check in commit step condition.Line 40 duplicates the no-fmt label check already present in the job-level condition (line 15), making this step condition overly restrictive. Since the job only runs when the label is not present, this additional check is unnecessary.
You can simplify the condition to:
- name: Commit Formatting Changes if: > - github.event.pull_request.head.repo.full_name == github.repository && - !contains(github.event.pull_request.labels.*.name, 'no-fmt') + github.event.pull_request.head.repo.full_name == github.repositoryThe fork detection (line 39) is the primary safeguard preventing unwanted commits. Keep that; the label check is already enforced at the job level.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/gofmt.yml(3 hunks).github/workflows/tests.yml(1 hunks)
🔇 Additional comments (2)
.github/workflows/tests.yml (1)
7-7: Verify that the master branch is being phased out before restricting tests to main only.This change restricts test runs to the main branch, preventing test execution on push to master. If the master branch is still in active use, this is a breaking change that could mask issues.
Please confirm that the master branch is no longer in use or is being actively phased out in your workflow/migration plan.
.github/workflows/gofmt.yml (1)
7-9: Permissions look correct.The added permissions (contents: write, pull-requests: write) are appropriate for the git-auto-commit-action and align with the workflow's needs to commit formatting changes to pull requests.
Simplified workflow conditions by removing redundancy: - Job-level: Removed unnecessary github.event_name check (workflow only triggers on PRs) - Step-level: Removed duplicate no-fmt label check (already enforced at job level) The workflow now has cleaner logic while maintaining the same behavior: - Runs on all PRs unless 'no-fmt' label is present - Auto-commits only to same-repo PRs (not forks)
gocanto
deleted the
claude/restrict-deployments-to-main-011CV5QhyZhPwEFxV2jY8KSo
branch
Summary by CodeRabbit