refactor: remove dead step functions, embed config structs, add fmt.Print archtest#103
Merged
Conversation
…, add fmt.Print archtest rule - Delete stepGitConfig, stepPresetSelection, stepPackageCustomization (orphaned from step→Plan/Apply refactor; only called by tests) - Remove the 8 corresponding test functions that covered dead paths - Embed InstallOptions and InstallState in Config so each field is defined once; adding a field now touches one struct, not three - Update all composite literals (snapshot_import, install_test, snapshot_test) to use embedded-struct syntax - Add internal/archtest/fmtprint_test.go: blocks new fmt.Print* calls to stdout in production code; 177 existing violations baselined
…ing Config plan.go writes opts.Preset during interactive preset selection. With embedding, returning &c.InstallOptions would let that mutation bleed back into the caller's Config. Return a copy instead — same semantics as before the refactor, embedding still eliminates field duplication in types.go.
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.
Summary
Three code-quality fixes from a review pass:
Dead step functions removed —
stepGitConfig,stepPresetSelection,stepPackageCustomizationwere orphaned by the step→Plan/Apply refactor. They existed in production files but were never called by production code — only by tests. Removed the functions and the 8 test functions that covered these dead paths.Config struct duplication eliminated —
Configpreviously duplicated every field fromInstallOptionsandInstallState.Confignow embeds both types; each field is defined exactly once. Adding a field now requires editing one struct, not three. Updated all composite literals to use embedded-struct syntax (Config{InstallOptions: InstallOptions{...}}).fmt.Printarchtest rule added — Production code has ~177 rawfmt.Print*calls that should eventually go throughui.*helpers (convention in CLAUDE.md). Without a mechanical check, every PR adds more. New rule ininternal/archtest/fmtprint_test.gobaselines existing violations and fails on any new ones added to non-exempt packages.Test plan
go vet ./...cleanmake test-unitpasses (all L1 including new archtest rule)fmt.Println→ fail, revert → green)grep stepGitConfig|stepPresetSelection|stepPackageCustomizationreturns zero results