Skip to content

fix: enforce gofmt in ci#2708

Closed
SkArchon wants to merge 1 commit intomainfrom
milinda/eng-9318-gofmt-enforcement
Closed

fix: enforce gofmt in ci#2708
SkArchon wants to merge 1 commit intomainfrom
milinda/eng-9318-gofmt-enforcement

Conversation

@SkArchon
Copy link
Copy Markdown
Contributor

@SkArchon SkArchon commented Mar 30, 2026

This PR

  • Fixes a few files which did not have gofmt applied
  • Adds ci checks so future files merged in will properly have go fmt applied

Summary by CodeRabbit

  • Style

    • Removed extra blank lines in test files for cleaner formatting.
    • Reordered imports across test files for consistency.
    • Adjusted test fixture formatting alignment.
  • Chores

    • Added code formatting check to GitHub Actions workflow to enforce gofmt standards.
    • Updated Makefile lint target to validate code formatting before running other linters.

Checklist

Open Source AI Manifesto

This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.

@SkArchon SkArchon marked this pull request as ready for review March 30, 2026 17:40
@SkArchon SkArchon requested a review from thisisnithin March 30, 2026 17:40
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 30, 2026

Walkthrough

This PR introduces Go code formatting validation via gofmt checks in the CI/CD pipeline (GitHub Actions and Makefile) while simultaneously correcting import ordering and removing extraneous whitespace across multiple test files to achieve compliance with formatting standards.

Changes

Cohort / File(s) Summary
CI/CD Formatting Checks
.github/actions/go-linter/action.yaml, router/Makefile
Added gofmt formatting check step that validates file formatting and exits with status 1 if unformatted files are detected, gating subsequent linting steps (go vet and staticcheck).
Import Reordering
router-tests/lifecycle/shutdown_test.go, router-tests/modules/context_error_field_test.go, router-tests/modules/set_scopes_test.go, router-tests/modules/stream_receive_test.go, router-tests/telemetry/connection_metrics_test.go, router-tests/telemetry/metrics_log_export_test.go, router-tests/telemetry/stream_metrics_test.go, router-tests/testenv/testenv.go
Reordered imports to standardize relative positioning of testenv and testutils packages across test files; testenv now consistently appears before testutils in import blocks.
Whitespace Cleanup
router-tests/events/kafka_events_test.go, router-tests/events/nats_events_test.go, router-tests/events/redis_events_test.go, router-tests/observability/prometheus_test.go, router/pkg/trace/meter.go
Removed extraneous blank lines between import blocks, type declarations, and function bodies to improve code density and formatting consistency.
Test Fixture Formatting
router/pkg/mcpserver/server_test.go
Adjusted indentation alignment in test fixture map value without altering test logic or behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: enforce gofmt in ci' directly and accurately describes the main objective of the PR: adding gofmt enforcement to the CI pipeline, while also applying gofmt formatting to existing files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 30, 2026

Router-nonroot image scan failed

❌ Security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-c9328044fa15f2ec88e74819ebafed1b1da60d3a-nonroot

Please check the security vulnerabilities found in the PR.

If you believe this is a false positive, please add the vulnerability to the .trivyignore file and re-run the scan.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/actions/go-linter/action.yaml (1)

25-36: Run the gofmt check before go vet/staticcheck for fail-fast CI.

This check is good; moving it to the top of the step list will reduce wasted CI time and keep ordering consistent with the router/Makefile lint flow.

♻️ Proposed reorder
 runs:
   using: composite
   steps:
+    - name: Check code formatting
+      run: |
+        unformatted=$(gofmt -l .)
+        if [ -n "$unformatted" ]; then
+          echo "The following files are not formatted correctly:"
+          echo "$unformatted"
+          echo ""
+          echo "Run 'gofmt -w .' to fix formatting."
+          exit 1
+        fi
+      shell: bash
+      working-directory: ${{ inputs.working-directory }}
+
     - name: Run go vet
       run: go vet ./...
       shell: bash
       working-directory: ${{ inputs.working-directory }}

     - name: Run staticcheck linter
       uses: dominikh/staticcheck-action@v1.3.1
       with:
         version: 2025.1.1
         install-go: false
         working-directory: ${{ inputs.working-directory }}
-
-    - name: Check code formatting
-      run: |
-        unformatted=$(gofmt -l .)
-        if [ -n "$unformatted" ]; then
-          echo "The following files are not formatted correctly:"
-          echo "$unformatted"
-          echo ""
-          echo "Run 'gofmt -w .' to fix formatting."
-          exit 1
-        fi
-      shell: bash
-      working-directory: ${{ inputs.working-directory }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/actions/go-linter/action.yaml around lines 25 - 36, The gofmt check
step ("Check code formatting") runs too late; move that entire step block so it
executes before the go vet/staticcheck steps in the GitHub Actions job sequence
to fail-fast and match router/Makefile ordering, keeping the same run script,
shell and working-directory inputs unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/actions/go-linter/action.yaml:
- Around line 25-36: The gofmt check step ("Check code formatting") runs too
late; move that entire step block so it executes before the go vet/staticcheck
steps in the GitHub Actions job sequence to fail-fast and match router/Makefile
ordering, keeping the same run script, shell and working-directory inputs
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fff4978e-49cf-4b70-813b-d04820fdd6d1

📥 Commits

Reviewing files that changed from the base of the PR and between 63db070 and d8c7764.

📒 Files selected for processing (16)
  • .github/actions/go-linter/action.yaml
  • router-tests/events/kafka_events_test.go
  • router-tests/events/nats_events_test.go
  • router-tests/events/redis_events_test.go
  • router-tests/lifecycle/shutdown_test.go
  • router-tests/modules/context_error_field_test.go
  • router-tests/modules/set_scopes_test.go
  • router-tests/modules/stream_receive_test.go
  • router-tests/observability/prometheus_test.go
  • router-tests/telemetry/connection_metrics_test.go
  • router-tests/telemetry/metrics_log_export_test.go
  • router-tests/telemetry/stream_metrics_test.go
  • router-tests/testenv/testenv.go
  • router/Makefile
  • router/pkg/mcpserver/server_test.go
  • router/pkg/trace/meter.go
💤 Files with no reviewable changes (5)
  • router/pkg/trace/meter.go
  • router-tests/events/redis_events_test.go
  • router-tests/events/kafka_events_test.go
  • router-tests/events/nats_events_test.go
  • router-tests/observability/prometheus_test.go

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.12%. Comparing base (932f436) to head (d8c7764).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2708      +/-   ##
==========================================
+ Coverage   62.86%   63.12%   +0.25%     
==========================================
  Files         249      249              
  Lines       26726    26643      -83     
==========================================
+ Hits        16802    16819      +17     
+ Misses       8534     8447      -87     
+ Partials     1390     1377      -13     
Files with missing lines Coverage Δ
router/pkg/trace/meter.go 44.35% <ø> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SkArchon SkArchon changed the title fix: Enforce gofmt in ci fix: enforce gofmt in ci Mar 30, 2026
@SkArchon SkArchon closed this Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant