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

Update deps #769

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Update deps #769

merged 1 commit into from
Jul 26, 2023

Conversation

rukai
Copy link
Contributor

@rukai rukai commented Jul 25, 2023

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.

I used https://deps.rs/repo/github/scylladb/scylla-rust-driver to tell which deps are out of date.

This PR updates all the out of date dependencies except for the following which are part of the public interface and updating them would be a breaking change:

secrecy
num_enum
num-bigint
bigdecimal
strum

I can include those changes in this PR or a follow up PR if desired. (I dont know if main currently contains breaking changes or not)

I also left out the histogram crate because upgrading it seems tricky.

The only update that required code changes was base64

@rukai rukai force-pushed the update_deps branch 3 times, most recently from 1d9f03c to 442eb20 Compare July 25, 2023 01:26
@piodul piodul merged commit e4a8802 into scylladb:main Jul 26, 2023
@mykaul
Copy link
Contributor

mykaul commented Jul 26, 2023

Thanks @rukai for your contribution!

@rukai
Copy link
Contributor Author

rukai commented Jul 27, 2023

Oh a breaking release happened, guess I should have included those updates that would have been breaking changes.

Should I make a PR for them now so they get included in the next release? Or do you plan for non-breaking releases before then?

@piodul
Copy link
Collaborator

piodul commented Jul 28, 2023

@rukai I'm a little hesitant to bump those dependencies at this point. While the driver's API is not stable, we are trying to eventually get there, and the current status quo where we update the crates that appear in the public API from time to time will prevent us from reaching that goal. Moreover, I'm a little worried that such change might be painful to the current users - if they depend on those crates directly then they will be forced to update, and I'm not sure how much work it would be for them.

I have created #771 where I outlined an approach that we will probably take. The #745 (which I plan to review soon) is a step in the right direction, based on my cursory glance.

@rukai
Copy link
Contributor Author

rukai commented Jul 28, 2023

Using features like that sounds non-additive which is problematic. E.g. a dependency enables scylla/bigint_04 and your project enables scylla/bigint_05 would be impossible to compile

But otherwise I'm glad you are planning a proper solution here, I'll leave it to you.

@piodul
Copy link
Collaborator

piodul commented Jul 28, 2023

Yup, the goal is to make the features additive. Importing the same crate more than once with different versions by one crate is perfectly possible, see here. Apart from that, it is just a matter of appropriately adjusting the API on our side. I don't see any fundamental issues with this approach, but time will tell.

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.

3 participants