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

Re-export derives from serde #820

Merged
merged 1 commit into from Mar 27, 2017

Conversation

2 participants
@dtolnay
Copy link
Member

dtolnay commented Mar 21, 2017

Fixes #797. @oli-obk I am still not settled on whether this should be on or off by default but I am leaning toward off.

// # struct S { /* ... */ }
//
// The reason re-exporting is not enabled by default is that disabling it would
// be annoying for crates that provide handwritten impls or data formats. They

This comment has been minimized.

@oli-obk

oli-obk Mar 22, 2017

Member

that provide only handwritten impls

I'd say these are really rare. I think the usability of serde increases a lot if the serde crate reexports the derive macros by default.

//
// # Used like this:
// # #[cfg(feature = "serde")]
// # #[macro_use]

This comment has been minimized.

@oli-obk

oli-obk Mar 22, 2017

Member

I didn't know that worked (reexporting derives). That's awesome!

@dtolnay

This comment has been minimized.

Copy link
Member Author

dtolnay commented Mar 22, 2017

On crates.io, of the 438 crates that depend on serde >=0.9, 17% of them do not depend on serde_derive.

anterofit, app_units, arccstr, blob, bodyparser, bson, chrono, cleverbot_io, core-graphics, cryogen_prelude, cssparser, elastic, elastic_types_derive_internals, envy, euclid, eval, gelf, generic-array, geojson, gluon_language-server, hopper, hyper_serde, ipc-channel, iron-json-response, jfs, json-color, json_io, jsonrpc-lite, jsonrpc-macros, juniper, kubewatch, linked-hash-map, mage, mdbook, meval, microsalt, mysql, ndarray, objectid, ocean, offscreen_gl_context, plist, preferences, record-query, reqwest, rmp-serde, rmpv, rocket_contrib, ruma-identifiers, ruma-signatures, rust_decimal, serde-bench, serde-encrypted-value, serde_ignored, serde_json, serde-pickle, serde-protobuf, serde_test, serde-transcode, serde_urlencoded, serde_utils, serde-value, serde-xml-rs, serde_yaml, sharp_pencil, sidekiq, sidekiq-rs, spaceralk, string_cache, tera, toml, treeflection, treeflection_derive, uhttp_json_api, url_serde, uuid

Some broad categories are:

  • Data formats (bson, serde_json, serde_yaml, toml)
  • Only handwritten impls (chrono, linked-hash-map, uuid)
  • Adapters (serde-encrypted-value, serde_ignored)
  • General serde helpers (serde_utils, serde-value)
  • Serde interop for specific libraries (bodyparser, hyper_serde, rocket_contrib, url_serde)
  • Serde traits in the public API (hopper, jsonrpc-macros, kubewatch, mage, preferences)
  • Standard library data structures only (cleverbot_io, geojson)

And lots more miscellaneous situations. I think this shows that depending on serde but not serde_derive is a fairly common and diverse use case.

For comparison, pretty much the same fraction of crates (19%) depend on serde >=0.9 but not serde_json so I don't think the number by itself justifies enabling the re-export by default. All three of serde and serde_derive and serde_json provide well-factored and distinguishable functionality so it is fair to expect users to depend on what they need.

I think this change is a last resort for the use case explained in the comment - preserving sanity around a serde cargo cfg. I do not intend to push this as the recommended way to import derives.

@dtolnay dtolnay merged commit 71ccc57 into master Mar 27, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dtolnay dtolnay deleted the reexport branch Mar 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.