ci: run gofmt #99
ci: run gofmt #99
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
Walkthroughgofmt GitHub Action workflow refactored: triggers narrowed to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant WF as GHA Runner
participant Repo as Repo
Note over GH,WF: PR labeled flow (pull_request labeled)
Dev->>GH: Add label "style" to PR
GH-->>WF: Trigger workflow (pull_request:labeled)
WF->>Repo: checkout (repo/ref chosen from PR head)
WF->>WF: setup-go@v5
WF->>Repo: git ls-files '*.go' | xargs gofmt -w -s
WF->>WF: Dry-run check (always)
alt head repo equals repo AND changes detected
WF->>Repo: commit back to PR head branch (no branch creation)
else external fork or no changes
WF-->>GH: finish without committing
end
sequenceDiagram
autonumber
actor CI as CI Push
participant GH as GitHub
participant WF as GHA Runner
participant Repo as Repo
Note over GH,WF: Push to main flow (push: main)
CI->>GH: Push to main
GH-->>WF: Trigger workflow (push)
WF->>Repo: checkout (default repo/ref)
WF->>WF: setup-go@v5
WF->>Repo: git ls-files '*.go' | xargs gofmt -w -s
WF->>WF: Dry-run check (always)
alt changes detected
WF->>WF: create branch gofmt-fixes-YYYYMMDD-HHMMSS
WF->>Repo: commit & push branch
WF->>GH: create Pull Request from branch (HAS_CHANGES == true)
else no changes
WF-->>GH: finish without creating branch/PR
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing Touches🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/gofmt.yml (1)
41-51: Add fork guard, restrict committed files, and fallback email
- Guard the commit step to only run on same-repo branches:
if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}- Narrow
file_pattern: .to Go sources:
file_pattern: '**/*.go'- Provide a fallback for
commit_user_emailwhen the secret is unset:
commit_user_email: ${{ secrets.GUS_GH_EMAIL || '41898282+github-actions[bot]@users.noreply.github.com' }}- name: Commit Changes to PR + if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} uses: stefanzweifel/git-auto-commit-action@v6.0.1 with: commit_message: apply coding style fixes commit_options: '--no-verify' - file_pattern: . + file_pattern: '**/*.go' repository: . commit_user_name: GitHub Actions - commit_user_email: ${{ secrets.GUS_GH_EMAIL }} + commit_user_email: ${{ secrets.GUS_GH_EMAIL || '41898282+github-actions[bot]@users.noreply.github.com' }} commit_author: gocanto <gocanto@users.noreply.github.com> branch: ${{ github.head_ref }}
🧹 Nitpick comments (3)
.github/workflows/gofmt.yml (3)
5-5: Label-only trigger won’t run on new commits after labeling—confirm this is intendedWith
pull_request: types: [labeled], subsequent pushes to the PR branch won’t re-run the workflow unless the label is re-added. If you want it to also run on new commits while the label is present, consider expanding triggers and gating by label presence.Example:
on: pull_request: - types: [labeled] + types: [labeled, synchronize, reopened]And pair with the predicate in Line 13 suggestion below.
13-13: Make the label check resilient across event typesCurrent guard works only for
labeledevents. If you later addsynchronize/reopened, switch to checking the PR’s labels array.- if: github.event.label.name == 'style' + if: contains(join(fromJson(toJson(github.event.pull_request.labels)).*.name, ','), 'style') || (github.event_name == 'pull_request' && github.event.action == 'labeled' && github.event.label.name == 'style')
16-16: Use go.mod as the source of truth for Go version
Rather than hard-coding a specific Go version in the matrix, point setup-go at your go.mod to avoid CI drift when you bump Go.strategy: matrix: - go-version: [1.25.1] os: [ubuntu-latest] @@ - name: Install Go uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} + go-version-file: 'go.mod'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gofmt.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/gofmt.yml (1)
29-31: Nice bump to setup-go v5Upgrading to
actions/setup-go@v5is good; it has better caching and security defaults.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/gofmt.yml (1)
25-26: Checkout head repo/ref — good fix for forked PRs.This addresses the prior “checkout with github.head_ref breaks on forks” concern. Nice.
🧹 Nitpick comments (3)
.github/workflows/gofmt.yml (3)
5-5: Optionally also run on new commits after labeling (keep label-gate).Right now, gofmt won’t re-run on later pushes unless the label is re-applied. If you want it to run on new commits while the PR still has the label, add
synchronizeand gate by label presence instead of the label event name.Apply:
on: pull_request: - types: [labeled] + types: [labeled, synchronize] jobs: gofmt: - if: github.event.label.name == 'style' + if: contains(github.event.pull_request.labels.*.name, 'style')Also applies to: 13-13
35-35: Harden path handling (spaces/newlines) and no-file case for gofmt.Use NUL-delimited paths and avoid invoking gofmt with no args.
Apply:
- run: git ls-files '*.go' | xargs gofmt -w -s + run: git ls-files -z -- '*.go' | xargs -0 -r gofmt -w -s
51-53: Avoid conflicting author settings; keep a single source of truth.
commit_authoroverridescommit_user_name/commit_user_email. Drop the redundant email to reduce confusion.Apply:
commit_user_name: GitHub Actions - commit_user_email: ${{ secrets.GUS_GH_EMAIL }} commit_author: gocanto <gocanto@users.noreply.github.com>If you prefer the secret-based email, remove
commit_authorinstead and keep the name/email pair.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gofmt.yml(4 hunks)
🔇 Additional comments (1)
.github/workflows/gofmt.yml (1)
43-43: Same-repo guard for commits — LGTM.Prevents attempts to push to forks. Good safety.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/gofmt.yml (1)
27-33: Checkout logic looks correct for forks (addresses prior concern)Using head repo/ref for PRs and default for others resolves the earlier
github.head_refissue and avoids pushing to forks. LGTM.
🧹 Nitpick comments (5)
.github/workflows/gofmt.yml (5)
22-24: Avoid hardcoding Go version; derive from go.mod and enable cachingPinning 1.25.1 risks drift and tool churn. Use setup-go’s go-version-file to match the module’s declared Go and cache the toolchain.
- strategy: - matrix: - go-version: [1.25.1] + # Derive Go version from the repo + strategy: {} - name: Install Go uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} + go-version-file: 'go.mod' + cache: true ``` <!-- review_comment_end --> Also applies to: 34-37 --- `42-45`: **“Dry run” happens after writing; rename or reorder** You already formatted with `-w`, so this isn’t a dry run. Either check first with `gofmt -l` or just report the diff. ```diff - - name: Check for formatting changes (dry run) + - name: Report formatting changes run: | - git diff --exit-code || echo "::warning::Formatting changes would be applied in normal mode" + git diff --name-only | sed 's/^/changed: /' || true continue-on-error: true ``` <!-- review_comment_end --> --- `59-60`: **Fallback for commit email to avoid empty secret** If `GUS_GH_EMAIL` isn’t set, commits may have an empty email. Provide a default. ```diff - commit_user_email: ${{ secrets.GUS_GH_EMAIL }} + commit_user_email: ${{ secrets.GUS_GH_EMAIL || 'actions@github.com' }} ``` <!-- review_comment_end --> --- `10-12`: **Tighten permissions to least privilege** Consider job-level permissions: read for PR label checks; elevate to write only when you actually push/commit (same-repo PR or push path). Example: ```yaml permissions: contents: read pull-requests: read jobs: gofmt: permissions: contents: write pull-requests: write
63-93: If keeping the push path, add path filter and label for discoverabilityTo avoid running on pushes that don’t touch Go, filter to Go files; optionally auto-apply the style label to the created PR for consistency.
on: pull_request: types: [labeled] push: branches: - main + paths: + - '**/*.go'And add to Create Pull Request:
with: labels: style
Summary
styleTesting
go test ./...https://chatgpt.com/codex/tasks/task_e_68c0d6749f0083339385c1c58ec89425
Summary by CodeRabbit