Deepen selective package export and release-plan modules - #27
Merged
Merged
Conversation
Move the selective-export pipeline out of package create's RunE closure (manifest selection, closure/parent-container/CDI-ref inclusion, metadata-graph building, file filtering, CSV/metadata regeneration) into a new internal/packaging module with a single entry point, BuildSelectivePackage. cmd/package.go's RunE shrinks from ~370 lines of inline orchestration to a thin call into the new module; exportMetadata/exportedObject become aliases to their internal/packaging equivalents so the unrelated `package dependencies` subcommand keeps compiling unchanged. No flag, output, error-text, or file-format changes; no CI/docs impact. Adds table-driven tests for the new module in internal/packaging.
release plan's RunE had two ~170-line branches (full-deployment vs selective/tag mode) that duplicated the same per-target pipeline (filter missing-transitive, validate, render totals, write package and publish files, accumulate manifest-log rows), differing only in how the initial asset set is resolved. Add internal/release.BuildPlan, a composing entry point over the existing per-step functions in plan.go, parameterized by asset source and a few mode-specific hooks/labels. cmd/release.go's RunE now calls it once per mode and shrinks by about a third. Drops releaseAssetsToManifestLog from cmd/manifest_log.go, dead code now that BuildPlan builds ManifestLogAsset rows internally. No flag, output, error-text, or file-format changes; no CI/docs impact. Adds tests for BuildPlan covering both modes.
wrapParagraph compared word length using raw len(), which counts ANSI escape bytes. A short colored status like "found" was measured as longer than the column width and hard-split at a raw byte offset, slicing through the escape sequence (e.g. "foun"/"d" across two physical rows in the release plan dependency status table). Measure and accumulate using visibleLen instead, and only fall back to stripped plain text when a word's visible length genuinely exceeds the column width.
`release plan` was validating asset existence per target with one Lookup/GetConnectionByName API call per asset, and recomputing that same existence matrix twice whenever --verbose was on: once to render the dependency status table, again inside BuildPlan to annotate the package/publish files. - validateAssetsExistInTarget batches all non-Connection assets for a target into a single Lookup call instead of one call per asset, matching the batching pattern already used in internal/dependencies. - ValidateAssetsForTargets computes the per-target validation matrix once; cmd/release.go threads it through PlanOptions.Validations so BuildPlan's missing-transitive filter and package annotation reuse it instead of re-querying the target. For a 65-asset / 2-target selective plan with --verbose, this cuts the API round trips from ~260 down to 2.
Piped/redirected table output with no explicitly configured theme now renders as markdown (colorless GFM table, already safe for scripting) rather than plain ASCII borders. An explicitly configured theme other than markdown/gh still downgrades to plain when non-interactive, same as before. resolveTableStyle previously seeded style.Theme with the literal string "default" as its unset sentinel, which made effectiveTheme's unset-theme branch unreachable from the main CLI flow. It now leaves Theme empty so effectiveTheme can distinguish "nothing configured" from an explicit --theme default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two architecture deepening refactors surfaced by an architecture review, each in its own commit. Both are pure internal restructurings: no CLI flag, output, error-text, or file-format changes; no CI/CD or docs impact.
refactor(package): deepen selective export into internal/packaging
Moves the selective-export pipeline out of
package create's RunE closure (manifest selection, closure/parent-container/CDI-ref inclusion, metadata-graph building, file filtering, CSV/metadata regeneration) into a newinternal/packagingmodule with a single entry point,BuildSelectivePackage.cmd/package.go's RunE shrinks from ~370 lines of inline orchestration to a thin call into the new module;exportMetadata/exportedObjectbecome aliases to theirinternal/packagingequivalents so the unrelatedpackage dependenciessubcommand keeps compiling unchanged.Adds table-driven tests for the new module.
refactor(release): collapse plan-mode duplication into BuildPlan
release plan's RunE had two ~170-line branches (full-deployment vs selective/tag mode) duplicating the same per-target pipeline (filter missing-transitive, validate, render totals, write package/publish files, accumulate manifest-log rows), differing only in how the initial asset set is resolved.Adds
internal/release.BuildPlan, a composing entry point over the existing per-step functions inplan.go, parameterized by asset source and a few mode-specific hooks/labels.cmd/release.go's RunE now calls it once per mode and shrinks by about a third.Drops
releaseAssetsToManifestLogfromcmd/manifest_log.go, dead code now thatBuildPlanbuildsManifestLogAssetrows internally.Adds tests for
BuildPlancovering both modes.Verification
gofmt -s -l .,go build ./...,go vet ./...,go test ./...(incl.-raceon the two touched packages), andgolangci-lint run ./...all clean. CLI--helpoutput forpackage createandrelease planconfirmed byte-identical before/after.🤖 Generated with Claude Code
https://claude.ai/code/session_019KEFMYHnZwja4rcKZC5Pim