-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
ledger-tool: Move ledger output methods to output.rs #34595
ledger-tool: Move ledger output methods to output.rs #34595
Conversation
8e4f2ed
to
506aca7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #34595 +/- ##
=========================================
- Coverage 81.8% 81.8% -0.1%
=========================================
Files 824 824
Lines 222261 222261
=========================================
- Hits 181893 181864 -29
- Misses 40368 40397 +29 |
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.
Lgtm! I don't feel strongly, but if you want to import std::result::Result
in output.rs
, I'm happy to re-review.
506aca7
to
8e21099
Compare
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.
I rebased on tip of master to pick up latest, but only change was the extra commit to swap the "default Result"
Summary of Changes
We have a dedicated file for CLI output, so move the various output functions there.
There is probably work to be done to clean up these
output_*()
functions; however, I think that should be deferred for the sake of this PR as I have another PR that I would like to have consume this one.