Add duffle claim show --output
#815
Conversation
| }, | ||
| } | ||
|
|
||
| cmd.Flags().BoolVarP(&onlyBundle, "bundle", "b", false, "only show the bundle from the claim") | ||
| cmd.Flags().BoolVarP(&cmdData.OnlyBundle, "bundle", "b", false, "only show the bundle from the claim") |
There was a problem hiding this comment.
Why not make this show the bundle from the claim, but not necessarily only the bundle? Then users would mix -o and -b if they wanted to?
There was a problem hiding this comment.
A reason not to enable -o and -b to be used together is that it is not obvious what such a command should do.
Some options:
- If you think of each of these as a filter on what output should be produced, then specifying both gives you no output (because they filter for different parts of the claim, there is not intersection).
- Since of
-oand-bhas a specific output, using both gives you both (in some order). - Since
-bsays you want part of the bundle and-oselects an output listed in the bundle, both gives the specification of the output from the bundle in the claim (rather than the contents of the file).
We tend to think of the CLI behavior through the first mental model (filtering), so the use-case of combining the flags is very limited. However, if users did want to mix -o and -b, we should decide what that behavior should be and how to easily convey that to the user.
There was a problem hiding this comment.
I was also wondering if --output could be a string slice, and then print all the selected outputs ? 🤔
There was a problem hiding this comment.
I'm fine fine with either a string slice or the way it's implemented in the PR; both implementations fit our use case. I'll leave it to the repo owners to decide what the cli user experience should be.
|
@silvin-lubecki does this side of the PR look good? |
|
/brig run |
youreddy
force-pushed
the
pr/duffle-outputs
branch
from
9ddd4f7 to
a4c2465
Compare
|
The PR in cnab-go was merged, going to update the Gopkg.toml file to point to the updated version! |
|
@youreddy should we release cnab-go or are you planning on pinning to a commit #? I'd prefer a released version, but we can always update to that once we have a few more things in. |
|
@jeremyrickard, I was going to specify master but you're right we should do a released version. And yes to releasing cnab-go! |
youreddy
force-pushed
the
pr/duffle-outputs
branch
from
08b8e84 to
e170da4
Compare
|
/brig run |
- Bump to cnab-go released version where outputs are implemented. - Add --output flag to claim show. - Run flag validation in PreRunE hook. - Uses subtests to separate happy path from error cases. Signed-off-by: Leah Hanson <lhanson@pivotal.io>
astrieanna
force-pushed
the
pr/duffle-outputs
branch
from
e170da4 to
e1ca5d8
Compare
astrieanna
changed the title
duffle claim show --output [DO NOT MERGE]duffle claim show --output
|
@jeremyrickard /brig run ? |
|
/brig run |
|
@astrieanna sorry for the delayed response. Were you asking me to re-run it, or what "/brig run" does? Happy to explain what it does if you'd like! |
|
/brig run |
|
Thanks for the persistence on this @astrieanna and @youreddy, much appreciated. |
Part of #747
After making the code compile against the cnab-go library with outputs support implemented, this PR adds a
--outputflag toduffle claim show. This allows users to dump the contents of output files to stdout.cc @youreddy @jeremyrickard