-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Create Printer object to manage output #1292
Conversation
Signed-off-by: Terry Howe <[email protected]>
df4b1e6
to
63d4954
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1292 +/- ##
==========================================
+ Coverage 81.94% 82.02% +0.07%
==========================================
Files 83 83
Lines 4005 4022 +17
==========================================
+ Hits 3282 3299 +17
Misses 500 500
Partials 223 223 ☔ View full report in Codecov by Sentry. |
@@ -184,7 +185,7 @@ func pushManifest(cmd *cobra.Command, opts pushOptions) error { | |||
} | |||
return opts.Output(os.Stdout, descJSON) | |||
} | |||
status.Print("Pushed", opts.AnnotatedReference()) | |||
printer.Print("Pushed", opts.AnnotatedReference()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TerryHowe Is this blocking your implementation of ORAS Golang scripting? IMHO this output should be classified to metadata output and it's better to user metadata push handler rather than new printer:
_, displayMetadata := display.NewPushHandler("", nil, opts.Verbose) // avoiding formatting and TTY output
displayMetadata.OnCopied(&opts.Target)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm trying to develop here is a layer similar to logrus that doesn't know what is is printing, it just knows how to print. To me at least, ideally the format code would be a layer on top of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Any good idea to inject the writer in? The writer might come from STDOUT or cobra command output.
Signed-off-by: Terry Howe <[email protected]>
@TerryHowe I added a new oras/cmd/oras/internal/display/metadata/view/printer.go Lines 33 to 40 in 61e0241
It provides To reflect your changes in #1288, cobra.Command.OutOrStdout can be injected via below interface oras/cmd/oras/internal/display/metadata/view/printer.go Lines 27 to 31 in 61e0241
Please help you help review related changes in #1310 and let me know your comment on this design, thanks. |
@TerryHowe I have added another solution in #1314, please let me know if it meets your expectation. |
Signed-off-by: Terry Howe <[email protected]>
4417131
to
1db763f
Compare
What this PR does / why we need it:
Create a printer object to manage output