Skip to content

tools: decide whether the migration CLI de-duplicates its decoder mirror #1109

Description

@gaborage

The tools/migration CLI keeps byte-identical local copies of two decode hooks that live in internal/configdecode: numericToDurationGuardHookFunc (pre-existing) and, as of ADR-074, emptyStringToNumericGuardHookFunc. Both copies carried a comment claiming the CLI cannot import github.com/gaborage/go-bricks/internal/configdecode because it is a separate module.

That claim is false. Go's internal-visibility rule is import-path-prefix based, not module based: github.com/gaborage/go-bricks/tools/migration/internal/commands sits under the github.com/gaborage/go-bricks prefix, so the import is legal. Verified three times independently — adding the import and a call to configdecode.NumericToDurationGuardHookFunc builds cleanly both with the repo workspace and under GOWORK=off (which resolves the published github.com/gaborage/go-bricks from the module cache, where internal/configdecode ships inside the module zip).

ADR-074 corrected the comments to state the real situation: the copy is a choice, not a necessity. This issue is the decision that choice deserves.

Why it matters

Nothing gates the sync. There is no Makefile target, CI job, or test that compares the two implementations — the only link is prose asking a future maintainer to remember. The two test suites do not cross-check either: the CLI table is a strict subset of the framework table, so a framework-side change to the whitespace handling or the duration exemption leaves the CLI tests green while the behaviours diverge. make mutate cannot see it: divergence is not a mutant.

The stakes are real for the delivered-empty hook in particular — the CLI decodes tenants.yaml for fleet rollouts, so a drift means one tool accepts a config the other rejects.

Options

  1. Import and bind. Delete both mirrors, import internal/configdecode. Removes the drift surface entirely. Cost: a separately-released binary binds to a package with no compatibility guarantee, so a rename in the framework breaks the CLI build at its next dependency bump (loudly, at least).
  2. Stay mirrored, add a pinning test. Keep the copies and add a test that fails when they diverge — e.g. run both hooks over one shared table of cases and assert identical outcomes, or compare the function bodies as source. Cost: the copies remain, but drift stops being silent.
  3. Status quo. Prose only. Cheapest today; the failure mode is a silent behavioural split discovered by a user.

Option 2 is the smallest change that removes the silence; option 1 is the one that removes the duplication. Either is defensible — what is not is the current state, where the promise is unbacked.

References

  • ADR-074 — names the sync obligation and that nothing gates it
  • internal/configdecode/configdecode.go
  • tools/migration/internal/commands/common.go

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions