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

Support for databake, writeable, zerovec traits? #183

Open
sffc opened this issue Apr 24, 2024 · 3 comments
Open

Support for databake, writeable, zerovec traits? #183

sffc opened this issue Apr 24, 2024 · 3 comments

Comments

@sffc
Copy link

sffc commented Apr 24, 2024

The bstr crate has an optional dependency on serde to add Serialize/Deserialize impls.

Would you accept a pull request adding implementations of the following traits behind optional, non-default features?

Both of these crates live in the icu4x repository but are intended to be used outside the context of icu4x.

I believe bstr depending on databake, writeable, and zerovec is the correct direction: bstr defines a concrete type that implements the traits exported from databake, writeable, and zerovec, which implement the traits on standard library types.

One risk of the dependency going in this direction is that we haven't released these crates at 1.0 yet, so the traits may be subject to change, which can make it annoying on the bstr side. We could alternatively consider a dependency going in the other direction, at least to start, and then switch the dependency direction when we reach 1.0 of zerovec and writeable.

CC @Manishearth @hsivonen

@sffc sffc changed the title Support for writeable and zerovec traits? Support for databake, writeable, zerovec traits? Apr 24, 2024
@BurntSushi
Copy link
Owner

One risk of the dependency going in this direction is that we haven't released these crates at 1.0 yet

Separate from anything else, yeah, this is probably a hard blocker. serde is an ecosystem standard and is likely to be at or remain at 1.0 for a very long time. Perhaps forever. I don't think I can say the same about any of the crates you've proposed. Basically, public dependencies (e.g., zerovec) should be at least as conservative (or "mature" or whatever word you want to use) as the dependent (e.g., bstr). bstr is pretty mature at this point, and I have no plans for bstr 2.0. More than that, others are expected to use bstr as a public dependency. This in turn makes my approximate plan "hopefully never release a bstr 1.0." I don't think it has reached widespread use as a public dependency though, so it could probably be done without too much pain. But still, the point is, I really do not want to do any major version changes. And if I take a public dependency on crates that are still evolving, I will effectively be forced to do major version changes at the union of all major version changes of my dependencies. That is a rather severe change from the status quo plan.

@Manishearth
Copy link

I'm okay with having an optional dependency on bstr going the other way from those crates, it seems fine especially since those crates change more often

@BurntSushi
Copy link
Owner

I typed this comment out in #182 but realized it would be better here:

The #183 issue is harder though. You'll likely need to wind up defining your own type to deal with those cases anyway, in order to get around the orphan rules. So if you're doing that anyway, it might not make sense to also use bstr? Not sure. With that said, bstr is meant to be a public dependency. It's encouraged to impl traits for BString and BStr. So you could convince the crates in #183 to add a bstr feature. If and when they reach "maturity," I'd be open to flipping that relationship around.


Which I think is consistent with what @Manishearth just said. :-)

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

3 participants