Show params with sources in porter explain output#3604
Draft
kichristensen wants to merge 2 commits into
Draft
Conversation
Parameters with a source (e.g. output injection) were hidden from `porter explain`. Show them with an `(injected)` default label so users know Porter will attempt to populate them from a prior run's output. For install and stateless actions where injection is impossible, the injected flag is false and no label is shown. Closes getporter#2905, getporter#2004 Signed-off-by: Kim Christensen <kimworking@gmail.com>
Update persisting-data guide to reflect that parameters with sources now appear in `porter explain` with an `(injected)` default label. Signed-off-by: Kim Christensen <kimworking@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates porter explain to display parameters that use CNAB parameter sources (e.g., sourced from prior outputs), including a new injected flag in JSON/YAML output and an (injected) indicator in the plaintext table output.
Changes:
- Stop filtering out parameters with sources from
porter explainoutput. - Add
injectedto the explain JSON/YAML schema and show(injected)in table output where applicable. - Update explain-related golden testdata and authoring docs to reflect the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/porter/explain.go | Includes sourced parameters in explain output; adds injected flag and table rendering behavior. |
| pkg/porter/testdata/explain/params-bundle.json | Adds parameter-sources extension + a sourced parameter for explain test fixtures. |
| pkg/porter/testdata/explain/expected-table-output.txt | Updates expected plaintext explain output to include sourced parameter and (injected) label. |
| pkg/porter/testdata/explain/expected-json-output.json | Updates expected JSON explain output to include the new injected field. |
| pkg/porter/testdata/explain/expected-yaml-output.yaml | Updates expected YAML explain output to include the new injected field. |
| docs/content/docs/development/authoring-a-bundle/persisting-data.md | Documents visibility of sourced parameters in porter explain and the (injected) label. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+463
to
+466
| defaultVal := fmt.Sprintf("%v", p.Default) | ||
| if p.Injected && (defaultVal == "<nil>" || defaultVal == "") { | ||
| defaultVal = "(injected)" | ||
| } |
Comment on lines
+321
to
+332
| // isInstallOrStatelessAction returns true when Porter cannot inject parameter | ||
| // values from previous outputs (install has no prior run; stateless actions | ||
| // don't track state). | ||
| func isInstallOrStatelessAction(bun cnab.ExtendedBundle, action string) bool { | ||
| if action == cnab.ActionInstall { | ||
| return true | ||
| } | ||
| if a, ok := bun.Actions[action]; ok && a.Stateless { | ||
| return true | ||
| } | ||
| return false | ||
| } |
Comment on lines
243
to
247
| for p, v := range bun.Parameters { | ||
| v := v // Go closures are funny like that | ||
| if bun.IsInternalParameter(p) || bun.ParameterHasSource(p) { | ||
| if bun.IsInternalParameter(p) { | ||
| continue | ||
| } |
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
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.
What does this change
Parameters with a
sourcedefined (e.g. an output source) were invisible inporter explainoutput. They are now shown with(injected)as the default value, indicating Porter will attempt to populate them from a prior run's output.For
installand stateless actions — where there is no prior run to inject from — the parameter is shown without the(injected)label.A new
injectedboolean field is included in JSON/YAML output.Example:
What issue does it fix
Closes #2905
Closes #2004
Notes for the reviewer
The
Injectedflag isfalse(and no label shown) when the action isinstallor stateless, since Porter cannot inject values in those cases.Checklist