build/devenv: Review feedback from earlier pull requests.#1148
Draft
winder wants to merge 6 commits into
Draft
Conversation
This was referenced Jun 2, 2026
winder
changed the title
winder
changed the title
winder
force-pushed
the
will/devenv-cli-improvements
branch
from
2d25bcd to
a81195f
Compare
Feedback from #1133 Add devenvruntime.DecodeConfig[T] to replace the identical marshal/unmarshal boilerplate repeated across all 10 component decode functions; also replace stringSlicesEqual with slices.Equal per review feedback. Each component's decode function is now a single typed call plus its own version check, removing ~80 lines of duplicated code.
Feedback from #1137 Move omitempty from a dead validate tag into the toml tag for NodeSets (toml:"nodesets,omitempty") and remove the stale "fallback" reference from the NewEnvironmentWithRegistry doc comment, which was removed in an earlier PR.
Feedback for #1133 Add an early nil check to DecodeConfig so that a missing config section (raw == nil) returns the zero value of T without error, matching the guard that was required on the original per-component decode functions.
Feedback for #1138 Tighten the phased runtime version check to required exact-match using CheckConfigVersion, update tests to supply version=1, and add tests for absent and wrong versions; also fix a stale topology path and grammar error in env-phased.toml and a ccv command typo in the CI workflow.
Update the LoadOutput version-mismatch error to list both supported versions (0 legacy, 1 phased), extract a pure stripPrivateKeys helper from storePhasedOutput, and fix the test to call it directly instead of re-implementing the stripping loop (which also had a k[0] panic risk).
Rename the private configKey constant to Key in each component package and update all internal and cross-component callsites to use the typed constant instead of a raw string literal.
winder
force-pushed
the
will/pr-stack-followup
branch
from
d507ac7 to
66e9b96
Compare
|
Code coverage report:
|
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.
PR Chain
There are multiple open pull requests that feed into each other:
Description
Testing
Checklist
changelogdirectory)just lint fix- no new lint errorsjust generate- mocks and protobufs are up to date