Start using workspace dependencies #297
Conversation
|
This also needs testing with https://firefox-source-docs.mozilla.org/build/buildsystem/rust.html Would you be interested in taking a stab at this @adriantombu or would you prefer me or @nordzilla to try it. It may be a bit before we can get to it if it's us. |
|
We need to document how to do the local testing in https://github.com/projectfluent/fluent-rs/tree/main/docs |
|
|
||
| [workspace.dependencies] | ||
| criterion = "0.3" | ||
| fluent-langneg = "0.13" |
There was a problem hiding this comment.
Thought: I'm not sure why this isn't in this repo, or if we should migrate to icu4x. @zbraniecki do you know?
There was a problem hiding this comment.
fluent-langneg should migrate to icu4x. But I'm not confident that icu4x should be providing language negotiation.
In terms of moving that into this repo - sure!
Cargo.toml
Outdated
| iai = "0.1" | ||
| intl_pluralrules = "7.0.1" | ||
| rustc-hash = "1" | ||
| serde = { version = "1.0", features = ["derive"] } |
There was a problem hiding this comment.
Suggestion: I don't think features should be enabled/disabled at the workspace level, they should be done at the crate level.
| fluent-bundle = { version = "0.15.2", path = "../fluent-bundle" } | ||
| fluent-fallback = { version = "0.7.0", path = "../fluent-fallback" } | ||
| unic-langid = "0.9" | ||
| fluent-bundle.workspace = true |
There was a problem hiding this comment.
Praise: Yes, this is much nicer.
@gregtatum I can have a look at that, I have some free time currently 😁 |
|
I tested against the Firefox codebase locally successfully and added a documentation file to explain the different steps to achieve it. |
adriantombu
requested review from
gregtatum
and removed request for
nordzilla
|
This is still in my review queue, but I haven't had time to look at it yet. |
There was a problem hiding this comment.
This looks good to me! I didn't follow the vendoring process on my own, but it seems correct. Next time I do a release I can try it out and handle any fast follows if needed. I would like to get a quick once-over from @nordzilla before merging to make sure I'm not missing anything.
|
@adriantombu Thanks once again for the contribution! 🎉 |
As discussed in #282 (comment) here's a separate PR to test the ability to use workspace dependencies that were introduced in Rust 1.64.
From what I see in the Firefox Rust update policy it should be ok as this is the Rust version used in the latest stable release of Firefox (lucky us haha).