Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

ledger-tool: Use OutputFormat printer in program subcommand #34475

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

CriesofCarrots
Copy link
Contributor

Problem

The solana-ledger-tool program subcommand uses solana_cli_output::OutputFormat, but uses its own custom printer for no reason.

Summary of Changes

Use OutputFormat::formatted_string() to make usage more consistent
No functional changes, except that the order of fields for OutputFormat::JsonCompact changes slightly, due to using serde_json::to_value(item).unwrap().to_string() under the hood (instead of serde_json::to_string(&output).unwrap()). We don't guarantee the order of json fields, so this change should be non-breaking.

Copy link
Contributor

@steviez steviez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! FYI @dmakarov - I doubt we have anything automated that is consuming this json downstream, but figured I'd mention it anyways. And even if something is consuming it, it should be parsing the whole json object and querying based on fields, so this change shouldn't matter anyways

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Merging #34475 (9b065d1) into master (aaccbdd) will increase coverage by 0.0%.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #34475   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         819      819           
  Lines      220894   220894           
=======================================
+ Hits       180753   180834   +81     
+ Misses      40141    40060   -81     

@CriesofCarrots CriesofCarrots merged commit 74d02ac into solana-labs:master Dec 15, 2023
34 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants