chore: move breakingchanges, changelog, openapi, pointer to foas module#1310
Conversation
Extract the remaining reusable packages from tools/cli into the standalone tools/foas library module (step 3 of the cli/foas split): - Move breakingchanges, changelog, openapi, pointer (and their subpackages) from tools/cli/internal/ to tools/foas/ - Delete tools/cli/pkg/ entirely (thin wrappers superseded by foas) - Rewrite all importers and go:generate directives to tools/foas/* - Copy the minimal test fixtures the moved tests need into foas/test/data - go mod tidy to pull in the foas dependencies Dependency stays one-way (cli -> foas). Both modules build, vet, and pass their unit suites.
yelizhenden-mdb
force-pushed
the
CLOUDP-413371-3
branch
from
58f779a to
b817832
Compare
yelizhenden-mdb
requested review from
andmatei,
andreaangiolillo,
drinkbird,
maks-m-mongo-leaf,
matt-condon,
saisundar and
wtrocki
| ) | ||
|
|
||
| require ( | ||
| github.com/davecgh/go-spew v1.1.1 // indirect |
There was a problem hiding this comment.
[q] Have we tried releasing the foas module and importing it into the CLI? We want to make sure the CLI is always ready to release, in case we have to fix an OpenAPI issue. If we merge this PR without the foas module in the CLI, we'd be blocked from making an emergency release until the last PR of this migration is completed.
There was a problem hiding this comment.
No, not really. The CLI intentionally consumes the FOAS library via go.work / replace rather than a published tag, as they are co-developed in the same repo. Since the E2E tests are passing, I don't think there is any change to the CLI build output.
Additionally, there isn't a release workflow for the FOAS library yet, which will be handled as part of Step 4. After that, I plan to use the versioned library in the OpenAPI MCP implementation. Let me know if you have any suggestions or concerns regarding this approach
There was a problem hiding this comment.
If we merge this PR without the foas module in the CLI, we'd be blocked from making an emergency release until the last PR of this migration is completed.
Now I see the edited comment. Do you suggest including a published and tagged foas library in the CLI instead of using Go workspaces?
There was a problem hiding this comment.
Do you suggest including a published and tagged foas library in the CLI instead of using Go workspaces?
Yes, but you raised a fair point: the e2e tests are passing, so I assume that, when we build the CLI binary via make build, the compiler fetches the foas module even if it isn't released (using go workspace). Feel free to ignore my comment. Thanks!
2 participants
Summary
Step 3 of the
tools/cli→tools/foasmodule split. Extracts the remaining reusable packages fromtools/cli/internal/into the standalonetools/foaslibrary module.Previous steps
tools/foaslibrary module: chore: scaffold tools/foas library module #1308 (merged)apiversion: chore: move apiversion package into tools/foas #1309 (merged)Changes
breakingchanges,changelog(+outputfilter),openapi(+slice,sunset,filter,errors), andpointerfromtools/cli/internal/totools/foas/.tools/cli/pkg/entirely — the thin wrappers had no importers and are superseded by the publicfoasmodule.go:generatemockgen directives fromtools/cli/internal/{pkg}→tools/foas/{pkg}.internal/path segment, so their relativetest/datapaths needed one fewer../. Copied the minimal subset foas needs intofoas/test/data/(~1.6MB) rather than relocating the 225MB changelog corpus that cli's remaining tests still depend on.go mod tidyonfoas/go.modto pull in the new dependencies (oasdiff, afero, mock, strcase, …).Dependency direction stays one-way:
cli → foas.foasreferences nothing incli.Verification
Build,unit-testsande2e-testsof CLI passesNext
Branch 4 — CI & release wiring (rename
code-health-foascli.yml, addrelease-foas-lib.yml).