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

Release easy::Value as a separate (1.0?) crate #268

Closed
sunshowers opened this issue Nov 24, 2021 · 6 comments
Closed

Release easy::Value as a separate (1.0?) crate #268

sunshowers opened this issue Nov 24, 2021 · 6 comments
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations

Comments

@sunshowers
Copy link
Contributor

I'd love to expose toml_edit::easy::Value (identical to toml-rs's Value?) in some public APIs in guppy. I'm thinking it could maybe be released as a separate crate (say toml-value), to allow toml_edit to continue to iterate on its API without breaking compat.

Some thoughts:

  • this could be a stable API, I don't think it's changed in toml-rs in a while.
  • it also means that the easy module can go away, or just re-export toml-value.
@epage
Copy link
Member

epage commented Nov 25, 2021

Sorry for the delayed response.

This would definitely help towards a 1.0. I'm assuming this would be a complete drop-in replacement for toml-rs. This means we'd need errors and deserializers. We'd probably create newtypes of the ones in toml_edit.

The one area of concern is for people using both crates. like cargo. You could have two different toml_edits in your dependency tree with different behaviors. With rigor, you could avoid the parts of toml-value that touch toml_edit.

Options for improving this

  • toml-value has a feature flag for serialization, that is enabled by default. cargo could disable default features. The big question is if we can do this and keep it close enough to toml-rs for cargo's needs
  • toml-value is only the data model and toml_edit re-exports it. Again, we'd need to check what, if anything, we lose from this.

@sunshowers
Copy link
Contributor Author

yeah, I'm thinking that toml-value is just the data model, with an optional serde feature for Serialize and Deserialize support (so not a complete replacement for toml-rs, which also has a parser and serializer). I'd expect toml_edit to stay as it is.

@epage
Copy link
Member

epage commented Nov 25, 2021

@ordian what are your thoughts on splitting out a toml-value for other crates to use in their public API? toml_edit::easy would then just re-export this.

I've been using cargo-release for the releases which handles workspaces, so it shouldn't be an issue from the release process perspective.

@ordian
Copy link
Member

ordian commented Nov 25, 2021

It makes sense to me. Go for it :)

@epage epage added the C-enhancement Category: Raise on the bar on expectations label Nov 30, 2021
@epage
Copy link
Member

epage commented Jan 13, 2022

Originally, I hesitated on this from laziness (all of the serializable logic needed for try_from and try_into but I forgot about the Dsplay needing a toml formatter. That makes splitting out only the data model a lot more complicated while maintaining the toml-rs API.

@epage epage added the A-serde Area: Serde integration label Sep 23, 2022
@epage
Copy link
Member

epage commented Jan 13, 2023

With #340, toml_edit::easy::Value will be moving to toml::Value. Closing this in favor of that issue

@epage epage closed this as completed Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

3 participants