Skip to content
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

Add Output implementation for deserializing JSON #218

Closed
wants to merge 3 commits into from

Conversation

casey
Copy link
Collaborator

@casey casey commented Feb 7, 2024

I figured that cradle was probably lonely from not having a PR for a while, so I decided to help.

I'm running a bunch of commands (ord and bitcoin-cli) which return JSON on standard output.

This PR adds a Json struct, which wraps any type that implements deserialize, and deserializes the command's standard output into an instance of that type. It's kind of unfortunate to have an output type which is specific to JSON, and not something that can be used to deserialize anything, but as far as I know, serde doesn't provide a trait which allows you to abstract over serialization formats, so I think adding a struct for each serialization format is the way.

serde and serde_json are pretty heavy dependencies, and cradle currently has no dependencies other than rustversion, so it might be wise to put this behind an optional, default-off feature flag.

(You will notice that there are no tests, and I can thus hear you clutching your pearls, but I promise to write some if you think this is a good idea!)

@soenkehahn
Copy link
Owner

Hey, thanks for the PR! I think in general this would be very nice to have. And given that, for all I know, you're the only active user of cradle, I think we should definitely add this!

I agree that it's a very heavy dependency and should be behind a feature flag, if that works for you. (Would the documentation on docs.rs be built for all available features, or just the default features?)

And yes, some tests would be nice. Clutch, clutch!

@casey
Copy link
Collaborator Author

casey commented Nov 3, 2024

Closing, since I'm sadly no longer using cradle with JSON T_T

@casey casey closed this Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants