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

'json' feature should be called serde and serde-json should not be a dependency #58

Closed
barafael opened this issue Apr 12, 2023 · 2 comments

Comments

@barafael
Copy link
Contributor

barafael commented Apr 12, 2023

Hi, thanks for this crate, it's got great potential!

I don't quite understand the json feature. The way I think it is done:

The json feature enables dependencies for serde and serde-json. In the code, the serializers/deserializers are derived via serde, if the json feature is enabled. The serde-json crate is not actually used.

Here is how this could be done better (I tested locally):
The serde feature enables a dependency on serde with the derive feature, as before. In the code, the serializers/deserializers are derived, as before. The serde-json crate appears nowhere.

I think this is the conventional and idiomatic way to do this. Are there things I am overlooking here?

EDIT: renamed, was "'json' feature seems half baked" which is vague and unnecessarily insulting. Sorry :)

@wooorm
Copy link
Owner

wooorm commented Apr 12, 2023

@barafael barafael changed the title 'json' feature seems half baked 'json' feature should be called serde and serde-json should not be a dependency Apr 12, 2023
@wooorm
Copy link
Owner

wooorm commented Apr 27, 2023

Solved in bbb3119

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

No branches or pull requests

2 participants