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

Introduce central crate version management #1908

Merged
merged 5 commits into from
Mar 20, 2023

Conversation

mokeyish
Copy link
Contributor

With this commit, we can manage versions of all crates in one place.

https://doc.rust-lang.org/cargo/reference/workspaces.html#the-dependencies-table

@mokeyish mokeyish force-pushed the workspace_dependency_table branch 3 times, most recently from 2b2d87b to 6727a61 Compare March 17, 2023 16:37
@bluejekyll
Copy link
Member

I think the Rust MSRV has been the reason we haven't adopted this yet. Do you know what Rust version this requires?

@mokeyish
Copy link
Contributor Author

I think the Rust MSRV has been the reason we haven't adopted this yet. Do you know what Rust version this requires?

Yes, Maybe you are right. It seems require version 1.64

rust-lang/cargo#3931 (comment)

@mokeyish mokeyish force-pushed the workspace_dependency_table branch from 6727a61 to d932185 Compare March 17, 2023 18:51
Copy link
Member

@bluejekyll bluejekyll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good to me. I think we should come up with some policy on features flags though, we have them in both the root as well as the direct dependencies. I wonder if we should always only specify them in the target crate?

@bluejekyll
Copy link
Member

FYI, this bumps MSRV to 1.64, this was discussed in the discord channel and approved by @djc there.

@mokeyish mokeyish force-pushed the workspace_dependency_table branch from 436d4c3 to 5319d48 Compare March 20, 2023 12:24
@mokeyish
Copy link
Contributor Author

I have updated to remove all feature flags from workspace denpendencies.

If you have time, Please review this again. @bluejekyll @djc

@bluejekyll
Copy link
Member

Yeah, I think I like the features better on the target crate. That makes a lot more sense to me personally since we enable and disable features for different reasons in different crates. Thanks for making that change.

@mokeyish mokeyish force-pushed the workspace_dependency_table branch from 5fc3cab to d018ba8 Compare March 20, 2023 16:02
@bluejekyll
Copy link
Member

I'm going to merge, thanks for the PR!

@bluejekyll bluejekyll merged commit d6b2e8b into hickory-dns:main Mar 20, 2023
@mokeyish mokeyish deleted the workspace_dependency_table branch March 20, 2023 22:06
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

Successfully merging this pull request may close these issues.

2 participants