-
Notifications
You must be signed in to change notification settings - Fork 13
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
[chore] Update manifests post fea-rs merge #513
Conversation
fea-rs/Cargo.toml
Outdated
diff = { version = "0.1.12", optional = true } | ||
rayon = { version = "1.6", optional = true } | ||
serde = { version = "1.0.147", features = ["derive"], optional = true } | ||
serde_json = {version = "1.0.87", optional = true } | ||
thiserror = "1.0.37" | ||
clap = { version = "4.0.32", features = ["derive"], optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think several more can move to workspace if we follow the current practice of doing so as soon as more than one crate uses something:
- fontc also uses both clap and rayon so they should move
- serde is in the workspace
- ufo2fontir uses norad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue is that in fea-rs these are optional, whereas they aren't in the other crates, so I don't think this works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can combine workspace and optional, https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#inheriting-a-dependency-from-a-workspace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey great, TIL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just need a few more workspace entries
47e86b6
to
3ff15ef
Compare
This makes fea-rs use the workspace deps where possible, and makes fontbe use the local fea-rs. It also makes a few other updates to update paths & urls in fea-rs/Cargo.toml, and it bumps a few out of date dependencies.
3ff15ef
to
8316844
Compare
This makes fea-rs use the workspace deps where possible, and makes fontbe use the local fea-rs.
It also makes a few other updates to update paths & urls in fea-rs/Cargo.toml, and it bumps a few out of date dependencies.