Skip to content

chore: address exhaustruct lint issues#4268

Merged
chronark merged 2 commits intomainfrom
11-07-chore_address_exhaustruct_lint_issues
Nov 10, 2025
Merged

chore: address exhaustruct lint issues#4268
chronark merged 2 commits intomainfrom
11-07-chore_address_exhaustruct_lint_issues

Conversation

@chronark
Copy link
Collaborator

@chronark chronark commented Nov 7, 2025

What does this PR do?

This PR adds several exemptions to the golangci-lint configuration for specific types from external packages, particularly from the github.com/AfterShip/clickhouse-sql-parser/parser and database/sql packages. It also updates various configuration parameters in the codebase

Fixes # (issue)

Type of change

  • Chore (refactoring code, technical debt, workflow improvements)
  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

  • Run golangci-lint to verify it doesn't report false positives for the exempted types
  • Verify that the API and other services start correctly with the updated configuration parameters
  • Test the clickhouse query parser to ensure it works properly with the initialized stmt field

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Ran make fmt on /go directory
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

@vercel
Copy link

vercel bot commented Nov 7, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
dashboard Ignored Ignored Preview Nov 7, 2025 7:38pm
engineering Ignored Ignored Preview Nov 7, 2025 7:38pm

@changeset-bot
Copy link

changeset-bot bot commented Nov 7, 2025

⚠️ No Changeset found

Latest commit: d08bf8b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Collaborator Author

chronark commented Nov 7, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vercel vercel bot temporarily deployed to Preview – engineering November 7, 2025 10:34 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard November 7, 2025 10:34 Inactive
@chronark chronark marked this pull request as ready for review November 7, 2025 10:34
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

📝 Walkthrough

Walkthrough

Expanded golangci exhaustruct exclusion list, added ClickhouseAnalyticsURL field to API configuration, introduced ReadOnlyDSN to database config, extended keys service config with ClickHouse and related fields, and initialized stmt field in parser.

Changes

Cohort / File(s) Summary
Linter configuration
go/.golangci.yaml
Expanded exhaustruct exclude list with parser patterns from clickhouse-sql-parser package, database/sql null types, and unkey CLI command struct
Configuration field additions
go/apps/api/integration/harness.go
Added ClickhouseAnalyticsURL (empty string) to api.Config struct initialization in test harness
Database configuration
go/cmd/create-clickhouse-user/main.go
Explicitly set ReadOnlyDSN field to empty string in db.Config passed to db.New
Seed configuration
go/cmd/dev/seed/verifications.go
Added ReadOnlyDSN to db.Config and extended keys.Config with RateLimiter, RBAC, Clickhouse, Region, UsageLimiter, and KeyCache fields
Parser initialization
go/pkg/clickhouse/query-parser/parser.go
Initialize stmt field to nil in NewParser composite literal

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Configuration struct field additions are straightforward with explicit nil/empty initializations
  • Golangci exclusion list expansion is a homogeneous change (repetitive pattern)
  • Consider verifying field dependencies across configuration structs, particularly in go/cmd/dev/seed/verifications.go where multiple new fields are added to keys.Config

Possibly related PRs

Suggested labels

Bug

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ⚠️ Warning The PR description is a blank template with no actual content filled in by the author. The author must fill out the PR description with: (1) a summary of what the PR does and why, (2) the issue number it fixes, (3) the specific type of change, and (4) actual testing instructions instead of placeholder 'Test A/Test B'.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'chore: address exhaustruct lint issues' accurately summarizes the main changes, which involve resolving exhaustruct linter violations by adding exemptions to golangci-lint configuration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 11-07-chore_address_exhaustruct_lint_issues

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.

❤️ Share

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

@chronark
Copy link
Collaborator Author

chronark commented Nov 7, 2025

@ogzhanolguncu

@chronark chronark enabled auto-merge November 7, 2025 14:03
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

Thank you for following the naming conventions for pull request titles! 🙏

Copy link
Contributor

@ogzhanolguncu ogzhanolguncu left a comment

Choose a reason for hiding this comment

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

LGTM

@chronark chronark added this pull request to the merge queue Nov 10, 2025
@graphite-app
Copy link

graphite-app bot commented Nov 10, 2025

Movie gif. Robert Downey Jr as Tony Stark in Iron Man slides on sunglasses with a smooth grin on his face. He gives a thumbs up as people in business attire and military uniforms stand and clap behind him.  (Added via Giphy)

Merged via the queue into main with commit 6a3148c Nov 10, 2025
21 of 22 checks passed
@chronark chronark deleted the 11-07-chore_address_exhaustruct_lint_issues branch November 10, 2025 12:36
@graphite-app
Copy link

graphite-app bot commented Nov 10, 2025

Graphite Automations

"Post a GIF when PR approved" took an action on this PR • (11/10/25)

1 gif was posted to this PR based on Andreas Thomas's automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants