refactor: move all packages under pkg/ for v5#559
Conversation
|
Important Review skippedToo many files! This PR contains 269 files, which is 119 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (269)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
A proposal guys! Waiting for you feeback! |
| never imports cobra or interacts with CLI frameworks directly beyond pflag. | ||
|
|
||
| `*cobra.Command` is only accepted in `service/`, `fx/`, and CLI builder packages | ||
| (e.g. `storage/bun/migrate/`). Pure packages accept `*pflag.FlagSet` and |
There was a problem hiding this comment.
Would you prefer to depends only on standard flags package?
(From my memory, we can bind flags)
There was a problem hiding this comment.
Yes -- but I'm not sure what we loose :/
There was a problem hiding this comment.
Both are not interchangeable.
So, either we add an abstraction layer, either we just define cobra will be our by default cli builder (which is fine imho)
There was a problem hiding this comment.
I don't even know the alternatvies to Cobra, so I don't think I'm the best person to decide on that -- but to me it might be good enough to depend on it.
There was a problem hiding this comment.
Let's just stick to cobra if we don't have a specific reason to switch away?
There was a problem hiding this comment.
Yes, we don't have reasons to change I think.
And if it was the case, I guess this is easily automatable.
a55fcab to
14b39b5
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #559 +/- ##
==========================================
- Coverage 28.80% 25.83% -2.98%
==========================================
Files 175 180 +5
Lines 7071 7065 -6
==========================================
- Hits 2037 1825 -212
- Misses 4914 5130 +216
+ Partials 120 110 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Move all 13 top-level Go packages into pkg/ for a cleaner repository structure. All internal import paths updated from github.com/formancehq/go-libs/v5/<pkg> to github.com/formancehq/go-libs/v5/pkg/<pkg>. Also includes cleanup: - Fix .golangci.yml stale v4 CircuitBreaker path - Replace deprecated str.Contains with stdlib slices.Contains - Update TODO(v4) markers to TODO - Delete stale coverage.txt - Fix pgtesting timeout not being propagated to docker pool - Update docs/ARCHITECTURE.md to reflect new structure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused type aliases (Authenticator, KeySet) from authnfx - Remove unused `debug` parameter from S3ModuleFromFlags - Delete comment-only stub files in messaging/publish/circuit/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Standardize naming across all packages: cli.go -> flags.go since these files all serve the same purpose (flag definitions). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Generated mock files had stale package names from before the pkg/ move, causing import cycles and build failures with --build-tags it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
82ff0b5 to
4ef50ac
Compare
| otelMetricsKeepInMemory, _ := cmd.Flags().GetBool(OtelMetricsKeepInMemoryFlag) | ||
|
|
||
| return MetricsModule(ModuleConfig{ | ||
| func ConfigFromFlags(flags *flag.FlagSet) ModuleConfig { |
There was a problem hiding this comment.
in my mind the conversation about cobra meant we'd revert to pass around cmd *cobra.Command instead of flag.FlatSet.
Not a blocker though.
Summary
Full repository restructure for v5: all Go packages are now under
pkg/for a cleaner and more consistent organization.What changes
pkg/:authn/,cloud/,errors/,fx/,messaging/,observe/,query/,service/,storage/,testing/,transport/,types/,workflow/github.com/formancehq/go-libs/v5/<pkg>togithub.com/formancehq/go-libs/v5/pkg/<pkg>github.com/formancehq/go-libs/v5) remains unchangedCleanup included
.golangci.yml: fix stale v4 path toCircuitBreaker→ v5pkg/authn/oidc/verifier.go: replace deprecatedstr.Containswith stdlibslices.Contains, updateTODO(v4)→TODOcoverage.txt: delete stale file (v3 data)pkg/testing/platform/pgtesting/postgres.go: fix Docker timeout not being propagated —MaximumWaitingTimeandStatusCheckIntervalwere never passed todocker.Configuration, causing timeouts in CI when multiple PostgreSQL tests run in parallelResulting structure
See
docs/ARCHITECTURE.mdfor full documentation on dependency rules and conventions.Test plan
go build ./...passesgo vet ./...passesgolangci-lint run ./...passes (0 issues)go test ./...passes (all 21+ packages, including PostgreSQL integration tests)🤖 Generated with Claude Code