Skip to content

Conversation

@tolgaozen
Copy link
Member

@tolgaozen tolgaozen commented Oct 9, 2025

Summary by CodeRabbit

  • New Features

    • None
  • Chores

    • Simplified linting/formatting setup and streamlined build targets; removed legacy tooling module.
  • Build/Dependencies

    • Upgraded Go toolchain to 1.25.1 and refreshed developer/tooling dependency versions.
  • Refactor

    • Consolidated configuration structures and made return/error handling explicit; minor formatting cleanups.
  • Tests

    • Updated tests and assertions to match new config/layout and assertion style.
  • Performance

    • Minor optimizations in multi-error message allocation.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Walkthrough

Consolidates linting/tooling config, upgrades Go and tools, refactors embedded config fields into named fields, standardizes error wrapping and explicit returns, adjusts balancer/context key and Postgres plan-cache handling, removes the tools module, and applies many small formatting and micro-optimizations.

Changes

Cohort / File(s) Summary
Tooling & CI
/.golangci.yml, Makefile, buf.gen.yaml, /go.mod, /tools/*, /.github/workflows/*, sdk/go/grpc/go.mod
Centralize/simplify golangci config; switch Makefile targets to go tool wrappers, remove dotenv-linter target, adjust lint/test/format commands; buf plugins made explicit; bump Go to 1.25.1 and add a tool block; remove tools module and package; update GitHub Actions Go versions.
Config refactor & usage
/internal/config/config.go, /internal/config/config_test.go, /pkg/cmd/serve.go
Convert many anonymous/embedded config fields into named struct fields (e.g., Server.HTTP, Server.GRPC), update tests and callsites to new field paths (cfg.Server.NameOverride).
Error wrapping & message normalization
/internal/authn/openid/authn.go, pkg/attribute/attribute.go, pkg/balancer/picker.go, pkg/dsl/*, pkg/* (many)
Replace %v/%s with %w for error wrapping; replace nested strings.Replace with strings.ReplaceAll in error formatting; other consistent error/message formatting changes.
Balancer & context key
/internal/engines/balancer/balancer.go, pkg/balancer/builder.go
Remove options []grpc.DialOption field from Balancer; change Key constant type to unexported contextKey while preserving value.
Engines utils & tests
/internal/engines/utils.go, /internal/engines/utils_test.go, /internal/engines/expand.go
Remove production getDuplicates helper; add equivalent helper in tests; trim stray blank lines in expand helpers.
Explicit returns & small control-flow edits
/internal/invoke/invoke.go, pkg/* (graph, lexer, tuple, development, coverage, etc.)
Replace many naked returns with explicit returns of named results; minor lexer condition adjustment; make error-returning paths return explicit values.
Storage (memory & postgres) edits
/internal/storage/memory/*, /internal/storage/postgres/*
Memory bundle writer discards second return from txn.First; several functions change to explicit two-value returns; Postgres utils use explicit (version, err) returns; import/whitespace tweaks.
Postgres-specific adjustments
/pkg/database/postgres/postgres.go, pkg/database/postgres/xid8.go, pkg/database/postgres/postgres_test.go
Use config.RuntimeParams when setting/deleting plan_cache_mode; remove internal pgSnapshot type and its related test case.
Generated code micro-optimizations
/pkg/pb/base/v1/*.pb.validate.go, .../errors.pb.validate.go, .../service.pb.validate.go
Preallocate msgs slices in many Error() implementations (make(...,0,len(m))) to reduce reallocations; behavior unchanged.
Tests & test helpers
/pkg/testinstance/postgres.go, /internal/factories/database.go
Replace dot-imported gomega with explicit gomega usage and add explicit assertions; clarify factory returns.
Misc hygiene & small edits
multiple files (internal/servers/*, internal/storage/context/attributes.go, pkg/schema/loader.go, tools/*, etc.)
Whitespace/import grouping changes, switch filepath.HasPrefix -> strings.HasPrefix, remove stray blank lines, and other minor non-functional edits.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant Make as Makefile
  participant Tool as Go Tooling
  participant CI as GitHub Actions
  participant Serve as serve command
  participant CFG as Config loader
  participant BAL as Balancer

  Dev->>Make: make format / lint-all / release
  Make->>Tool: go tool gofumpt / go tool golangci-lint / go test
  Dev->>CI: push -> workflow (Go 1.25.1)
  Dev->>Serve: permify serve
  Serve->>CFG: Load config (named fields: Server.HTTP, Server.GRPC)
  Serve->>BAL: NewCheckEngineWithBalancer(cfg.Server.NameOverride)
  BAL-->>Serve: Engine instance
  Serve-->>Dev: Server started
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • tolgaOzen

Poem

A rabbit nibbles on the diff tonight,
Tools aligned and lint rules made bright.
Fields named tidy, errors wrapped with grace,
Tests now call gomega with a steady pace.
Thump-thump—small hops, cleaner code in sight! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately highlights the reorganization and updates to protobuf generation configuration, which is a real part of the changeset, even though the pull request also includes broader refactorings and tooling updates; it is specific and relates directly to notable changes in buf.gen.yaml and related configs.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/add-vtproto-support

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a90333 and 9336607.

📒 Files selected for processing (4)
  • .github/workflows/coverage.yml (1 hunks)
  • .github/workflows/nightly.yaml (1 hunks)
  • .github/workflows/release.yml (1 hunks)
  • sdk/go/grpc/go.mod (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Test with Coverage
  • GitHub Check: Analyze (go)
  • GitHub Check: Test with Coverage

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.

@tolgaozen tolgaozen merged commit fd2e87b into master Oct 10, 2025
12 of 14 checks passed
@tolgaozen tolgaozen deleted the feature/add-vtproto-support branch October 10, 2025 00:13
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.

2 participants