Conversation
WalkthroughMostly import-path updates switching internal libs to their v3 module paths; one functional change in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Dav-14
force-pushed
the
feat/upgrade-go-libs
branch
from
8dae28d to
c7535da
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pkg/organization.go (1)
50-53: Harden the capabilities type assertion to avoid potential panics
value.([]interface{})will panic if the capability’s underlying type ever changes (or isnil), which makes this path brittle for server‑side changes.Consider guarding the assertion and surfacing a clearer error instead of panicking:
- if value, ok := capabilities[key]; ok { - if values := value.([]interface{}); len(values) > 0 { - if !checker(values) { - return fmt.Errorf("unsupported membership server version: %s", value) - } - } - } + if value, ok := capabilities[key]; ok { + values, ok := value.([]interface{}) + if !ok { + return fmt.Errorf("unexpected capability type for %q: %T", key, value) + } + if len(values) > 0 && !checker(values) { + return fmt.Errorf("unsupported membership server version: %v", value) + } + }Please verify this matches the actual shape of
region.Data.Capabilitiesand adjust the error handling if you prefer a softer failure (e.g., just skipping unknown/invalid types).cmd/ledger/import.go (1)
121-148: Optional: simplify bufio.Scanner error/EOF handlingInside the scan loop you call
scanner.Err()on every iteration and check it againstio.EOF, butbufio.Scannernever reportsio.EOFviaErr()—EOF is signaled byScan()returningfalse. A more idiomatic pattern is:scanner := bufio.NewScanner(f) for scanner.Scan() { // use scanner.Bytes() } if err := scanner.Err(); err != nil { return nil, fmt.Errorf("error reading file: %w", err) }This would remove the redundant
io.EOFcheck and the per-iterationscannerErrlogic while preserving behavior.cmd/ledger/internal/print.go (1)
191-195: Use the provided writer instead of stdout in PrintMetadata
PrintMetadatatakes anio.Writer, but in thelen(metadata) == 0branch it callsfmt.Println("No metadata."), which bypasses the supplied writer and always writes to stdout. This is inconsistent with the rest of the function.Consider:
- fmt.Println("No metadata.") + fmt.Fprintln(out, "No metadata.")to keep all output routed through
out.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (34)
cmd/cloud/apps/list.go(1 hunks)cmd/cloud/apps/runs/list.go(1 hunks)cmd/cloud/apps/variables/list.go(1 hunks)cmd/cloud/apps/versions/list.go(1 hunks)cmd/cloud/organizations/authentication-provider/configure.go(1 hunks)cmd/cloud/organizations/create.go(1 hunks)cmd/cloud/organizations/oauth-clients/create.go(1 hunks)cmd/cloud/organizations/oauth-clients/update.go(1 hunks)cmd/ledger/accounts/set_metadata.go(1 hunks)cmd/ledger/accounts/show.go(1 hunks)cmd/ledger/create.go(1 hunks)cmd/ledger/import.go(1 hunks)cmd/ledger/internal/print.go(1 hunks)cmd/ledger/send.go(1 hunks)cmd/ledger/transactions/list.go(1 hunks)cmd/ledger/transactions/num.go(1 hunks)cmd/ledger/transactions/revert.go(1 hunks)cmd/ledger/transactions/set_metadata.go(1 hunks)cmd/orchestration/triggers/create.go(1 hunks)cmd/payments/connectors/configs/getconfig.go(1 hunks)cmd/profiles/setdefaultorganization.go(1 hunks)cmd/profiles/setdefaultstack.go(1 hunks)cmd/root.go(2 hunks)cmd/stack/create.go(1 hunks)cmd/stack/modules/list.go(1 hunks)cmd/stack/update.go(1 hunks)cmd/stack/upgrade.go(1 hunks)pkg/clients.go(1 hunks)pkg/metadata.go(1 hunks)pkg/organization.go(1 hunks)pkg/printer/log.go(1 hunks)pkg/profile.go(1 hunks)pkg/stack.go(1 hunks)pkg/versions.go(1 hunks)
🔇 Additional comments (35)
pkg/profile.go (1)
23-23: Logging import path bump looks correct; ensure v3 API compatibility and module updateThe switch to
github.com/formancehq/go-libs/v3/loggingis consistent with the PR’s goal and matches how logging is used in this file (logging.FromContext(ctx).Debug/Debugf). AssumingFromContextand the returned logger’s interface are unchanged in v3, this should be a drop‑in upgrade. Please double‑check that:
go.modrequires the appropriategithub.com/formancehq/go-libs/v3version, andgo test ./...(and any relevant integration checks) pass, confirming there are no subtle behavior changes in the logging package.pkg/printer/log.go (1)
9-9: Import path update to v3 is correct and consistent across the codebase.The change from
github.com/formancehq/go-libs/timetogithub.com/formancehq/go-libs/v3/timeat line 9 is correct. Verification confirms all 36 go-libs imports across the codebase (api, collectionutils, logging, metadata, pointer, time) have been consistently upgraded to v3, with no unversioned imports remaining.pkg/clients.go (1)
13-13: Verify go-libs v3 logging API compatibility before merging.The import path update from
github.com/formancehq/go-libs/loggingtogithub.com/formancehq/go-libs/v3/logginginvolves a major version bump. While the current usage (logging.Debug()at line 38) is simple, v3 as a major release can introduce breaking changes. Confirm that the v3 logging package maintains theDebug()method signature used in this file by checking the go-libs v3 release notes or release diff.pkg/metadata.go (1)
11-11: Import upgrade to v3 metadata package is correct.The import path has been properly updated to use the v3 module version. The
metadata.Metadatatype maintains full API compatibility, supporting map-like operations (initialization, indexing, length checks, and range iteration) as used throughout the file.pkg/organization.go (1)
8-8: Import upgrade togo-libs/v3/collectionutilslooks correct; verify module usage is consistentThe path change to
github.com/formancehq/go-libs/v3/collectionutilsmatches the v3 module layout and keeps the existingReducecall intact. Before merging, confirm that:
go.modrequiresgithub.com/formancehq/go-libs/v3(not v2 or legacy versions)- No remaining imports of the v2
github.com/formancehq/go-libs/collectionutilsexist elsewhere in the codebase- All other files that use collectionutils are updated consistently to v3
cmd/profiles/setdefaultstack.go (1)
10-10: Import path migration tocollectionutilsv3 looks correctOnly the import path was changed;
collectionutils.Reduceusage instackCompletionremains consistent, and no other issues are apparent in this file.Please confirm CI /
go test ./...passes withgithub.com/formancehq/go-libs/v3/collectionutilsdeclared ingo.mod.cmd/stack/upgrade.go (1)
11-11: Pointer helper import updated to v3 consistentlyThe import now targets
github.com/formancehq/go-libs/v3/pointer, andpointer.For(specifiedVersion)is still used correctly to populatereq.Version; no additional issues spotted in this file.Please ensure the v3 pointer module is present in
go.modand that the membership client still expects a*stringforStackVersion.Version.cmd/stack/modules/list.go (1)
7-7: Time wrapper import migrated to v3 without logic changesImporting
github.com/formancehq/go-libs/v3/timeand usingtime.Time{Time: module.LastStateUpdate}inRenderremains consistent with the existing pattern; no new issues identified.Please confirm the generated membership client’s
LastStateUpdate/LastStatusUpdatetypes are still compatible with the v3time.Timewrapper.cmd/profiles/setdefaultorganization.go (1)
10-10: Collectionutils import upgrade appears safeThe switch to
github.com/formancehq/go-libs/v3/collectionutilskeeps theReducecall inorganizationCompletionidentical in structure; no other changes in this file.Please verify the project builds cleanly with the v3 collectionutils dependency (e.g., via CI or
go build ./...).cmd/cloud/organizations/create.go (1)
6-6: Pointer import migrated to v3 for organization creation
pointer.For(domain)still correctly supplies a*stringfororgData.Domain; the only change is the import path togo-libs/v3/pointer.Please confirm the membership client’s
OrganizationData.Domainfield type still matches the pointer helper’s return type after the v3 upgrade.cmd/cloud/apps/list.go (1)
7-7: Apps listing now uses v3 pointer helperThe pagination parameters still use
pointer.For(int64(page))andpointer.For(int64(pageSize)); only the import was updated togo-libs/v3/pointer.Please ensure the deploy server client still expects
*int64for these arguments and that builds/tests pass with the v3 pointer dependency.cmd/ledger/create.go (1)
12-12: Ledger create command correctly switched to v3 pointerThe
Bucketfield is still populated viapointer.For(...); the import now points togithub.com/formancehq/go-libs/v3/pointer, with no surrounding logic changes.Please confirm the Formance SDK
V2CreateLedgerRequest.Buckettype is still compatible with the v3 pointer helper return type.pkg/stack.go (1)
8-8: Collectionutils import in stack helpers updated cleanly to v3
collectionutils.Reduceusage inStackCompletionis unchanged apart from the import path, and the rest of the file is unaffected.Please make sure the v3 collectionutils module is referenced in
go.modand that auto-completion behavior is validated via existing tests or manual checks.cmd/cloud/organizations/oauth-clients/update.go (1)
9-9: Import path bump to go-libs/v3/pointer looks correctThe switch to
github.com/formancehq/go-libs/v3/pointeris consistent with the rest of the PR and keepspointer.Forusage intact, so behavior should be unchanged as long asgo.modreferences the v3 module.Please ensure
go mod tidyandgo test ./cmd/cloud/organizations/oauth-clients/...pass after the module upgrade.cmd/ledger/transactions/num.go (1)
13-13: Collectionutils v3 import aligns with existing usageUpdating to
github.com/formancehq/go-libs/v3/collectionutilswhile keepingConvertMap(..., collectionutils.ToAny[string])as-is should be a no-op behavior-wise, assuming the v3 API is compatible.After updating
go.modto the v3 module, please rungo test ./cmd/ledger/transactions/...to confirm there are no type or import resolution issues.cmd/ledger/import.go (1)
18-18: Pointer import upgraded to v3 without behavior changeUsing
github.com/formancehq/go-libs/v3/pointerwithpointer.For[int64](1)forPageSizekeeps the request semantics the same; this looks consistent with the rest of the migration.Confirm
go.modincludes the v3 module and thatgo test ./cmd/ledger/...passes so the genericpointer.Forusage still type-checks as expected.cmd/ledger/transactions/revert.go (1)
7-7: Revert command pointer import migration looks soundSwitching to
github.com/formancehq/go-libs/v3/pointerwhile keepingpointer.For(true)forAtEffectiveDatepreserves the request shape; no behavioral changes expected.Please verify that the new go-libs v3 dependency is present in
go.modand thatgo test ./cmd/ledger/transactions/...passes.cmd/orchestration/triggers/create.go (1)
11-11: Pointer v3 import in trigger creation is consistentUsing
github.com/formancehq/go-libs/v3/pointerforpointer.For(filter)keeps the optionalFilterbehavior intact; the change is limited to the module path.After updating dependencies, rerun
go test ./cmd/orchestration/triggers/...to ensure the v3 pointer API remains compatible.cmd/ledger/internal/print.go (1)
13-13: Collectionutils v3 import matches existing usageThe move to
github.com/formancehq/go-libs/v3/collectionutilskeepsMapandConvertMap(..., ToFmtString)usage the same, so behavior should be unchanged with a compatible v3.Please confirm
go test ./cmd/ledger/internal/...passes with the upgraded dependency.cmd/ledger/accounts/show.go (1)
11-11: Accounts show: collectionutils v3 import looks goodUpdating to
github.com/formancehq/go-libs/v3/collectionutilswhile keepingConvertMap(c.store.Account.Metadata, collectionutils.ToFmtString)should be behaviorally identical with a compatible v3.Once
go.modreferences the v3 module, please rungo test ./cmd/ledger/accounts/...to double-check for any API drift.cmd/cloud/apps/variables/list.go (1)
9-9: Variables list: pointer v3 import is consistent with usageThe change to
github.com/formancehq/go-libs/v3/pointerkeeps the paging parameters wrapped viapointer.For(int64(...)), so the request contract remains the same.After updating dependencies, please verify with
go test ./cmd/cloud/apps/variables/...(and/or a quick manual run) that paging still behaves as expected.cmd/ledger/transactions/set_metadata.go (1)
8-8: LGTM!The import path has been correctly updated to use the v3 module, and the usage of
collectionutils.ConvertMapandcollectionutils.ToAnyremains unchanged.cmd/cloud/apps/runs/list.go (1)
9-9: LGTM!The import path has been correctly updated to use the v3 module, and the usage of
pointer.Forremains unchanged.cmd/cloud/organizations/oauth-clients/create.go (1)
6-6: LGTM!The import path has been correctly updated to use the v3 module, and the usage of
pointer.Forremains unchanged.cmd/cloud/organizations/authentication-provider/configure.go (1)
9-9: LGTM!The import path has been correctly updated to use the v3 module, and the usage of
pointer.Forremains unchanged.cmd/ledger/accounts/set_metadata.go (1)
8-8: LGTM!The import path has been correctly updated to use the v3 module, and the usage of
collectionutils.ConvertMapandcollectionutils.ToAnyremains unchanged.cmd/cloud/apps/versions/list.go (1)
10-10: LGTM!The import path has been correctly updated to use the v3 module, and the usage of
pointer.Forremains unchanged.cmd/payments/connectors/configs/getconfig.go (1)
12-12: Import path is correct and API is compatible.The v3 collectionutils module is already used throughout the codebase with multiple functions (Filter, Reduce, Map, ConvertMap, Contains) in active use, confirming API compatibility. This import path upgrade aligns with the broader v3 libs upgrade effort across the project.
cmd/stack/update.go (1)
7-7: Import path upgrade looks good.The import path has been correctly updated to the v3 module. However, the pointer package API compatibility cannot be definitively verified due to conflicting information—while the code consistently uses
pointer.For()throughout the codebase, automated searches could not confirm the package's existence or its API stability in v3. Manual verification of the formancehq/go-libs v3 documentation or release notes may be needed to ensure the For function is compatible with the v3 release.cmd/stack/create.go (1)
12-12: LGTM! Import path updated to v3.The migration from unversioned to v3 pointer package is correct, and usage remains consistent throughout the file.
cmd/ledger/transactions/list.go (1)
13-14: LGTM! Import paths updated to v3.Both collectionutils and pointer packages have been correctly migrated to v3 with consistent usage throughout the file.
cmd/ledger/send.go (1)
11-11: LGTM! Import path updated to v3.The collectionutils package has been correctly migrated to v3 with consistent usage.
cmd/root.go (2)
16-18: LGTM! Import paths updated to v3.The api, logging, and service packages have been correctly migrated to v3.
109-109: The code reviewed does not match the description. Line 109 incmd/root.gocontainscmd := NewRootCommand(), notservice.Execute(). Noservice.Execute()call exists in the file. The review is addressing non-existent code.Likely an incorrect or invalid review comment.
pkg/versions.go (1)
9-9: LGTM! Import upgrade to v3 is syntactically correct.The import path upgrade to
github.com/formancehq/go-libs/v3/collectionutilsis valid and consistent with the PR objectives. The usage at line 42 (collectionutils.Contains(serverInfo.Capabilities, capability)) follows the standard two-argument pattern for this utility function.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/root.go (1)
74-77: Logger initialization wiring is reasonable; consider env and case handling for JSON mode
logging.NewDefaultLogger(cmd.OutOrStdout(), service.IsDebug(cmd), fctl.GetString(cmd, fctl.OutputFlag) == "json", false)correctly ties:
- debug mode to
service.IsDebug(cmd), and- JSON logging to the
outputflag value"json".Two minor behavioral nuances to be aware of:
fctl.GetStringonly falls back to theOUTPUTenv var when the flag value is empty, but the flag’s default is"plain", so the env var won’t affect JSON mode unless you change the defaulting strategy.- The equality check is case-sensitive, so
--output JSONorOUTPUT=JSONwould not enable JSON logging.If you want env/flag values like
"JSON"or mixed-case to work, or you wantOUTPUTto override the default, you could normalize the value (e.g.,strings.ToLower(...)) and/or handle the default separately.Please confirm that the intended UX is: JSON logging only when
--output json(exact lowercase) is passed, and that you don’t needOUTPUTenv or case-insensitive handling here. If needed, we can adjust the expression accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cmd/root.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/root.go (1)
pkg/flags.go (2)
GetString(35-41)OutputFlag(17-17)
🔇 Additional comments (1)
cmd/root.go (1)
16-18: go-libs/v3 import updates look consistent with usageThe updated imports for
api,logging, andservicealign with how they’re used later in this file (e.g.,api.ErrorResponse,logging.ContextWithLogger,service.IsDebug). Assuminggo.modis pinned togithub.com/formancehq/go-libs/v3at the intended version, this looks good.Please double-check that
go.modandgo.sumare updated to the same go-libs v3 version you expect and rungo test ./...to confirm there are no hidden API mismatches.
1 participant
Summary by cubic
Upgrade go-libs to v3 and refresh dependencies for compatibility. Updated imports and fixed logger initialization to keep the CLI running smoothly.
Dependencies
Migration
Written for commit c4a463c. Summary will update automatically on new commits.