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

Bump num_enum to 0.6..=0.7 #857

Closed
wants to merge 3 commits into from
Closed

Conversation

oeb25
Copy link
Contributor

@oeb25 oeb25 commented Oct 31, 2023

This PR bumps num_enum from 0.5 and 0.6 to 0.6..=0.7. The motivation for this is that [email protected] transitively allows for [email protected].

Unfortunately another dependency of scylla, ntest, still depends on [email protected] in its current release, but has bumped to 0.20 on master. ntest is however only a dev-dependency, so I still think this will have upside for downstream users of scylla.


Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@oeb25 oeb25 changed the title Bump num_enum to 0.7 Bump num_enum to 0.7 Oct 31, 2023
The transitive dependency `[email protected]` has MSRV of 1.67.0, while
scylla has 1.66.0. So to maintain the current MSRV we specify a range
that works for `Cargo.lock.msrv`.

Curiously, without `Cargo.lock.msrv` is not able to resolve an
appropriate version of `toml_edit`, and fails.
@oeb25 oeb25 changed the title Bump num_enum to 0.7 Bump num_enum to 0.6..=0.7 Oct 31, 2023
`cargo check --locked` errors due to the new requirements of `num_enum`.

Running `cargo check` we see that we remove the dependency on
`[email protected]` and are left with only `[email protected]`.

This seems to be the only change to `Cargo.lock.msrv`, and should not
cause problems.
@piodul
Copy link
Collaborator

piodul commented Nov 2, 2023

I'm not sure it is a good idea to allow for two major versions. This will interact weirdly with the cargo resolver and could cause issues when used in a project where some dependencies (or the project itself) specifies num_enum = 0.6 and others num_enum = 0.7. See the recommendation here: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-requirements.

I'd rather stick to only one version (bumping to just 0.6 sounds like less of a hassle).

In the future, I'd like to get rid of the dependency altogether as it is not stable (<1.0, problematic for making this crate's API stable) but appears in the public interface (for Consistency and SerialConsistency) and doesn't offer too much functionality (we can implement TryFrom conversions manually).

@muzarski muzarski mentioned this pull request Feb 13, 2024
8 tasks
@piodul piodul closed this in #931 Feb 15, 2024
@oeb25 oeb25 deleted the bump-num-enum branch February 15, 2024 13:11
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