Bump dependencies, follow rustfmt and clippy

[ds-cbo]

Main reason for this PR is to bump strum to 0.25 so we can get rid of the syn 1.x dependency and move to syn 2.x

While working on it and running tests, cargo fmt and cargo clippy had several remarks, so I followed those as well

Most notably lazy_static! has been replaced with once_cell as preferred by clippy

[codecov]

Codecov Report

Attention: 74 lines in your changes are missing coverage. Please review.

Comparison is base (226b6fb) 71.39% compared to head (9a26c8d) 70.91%.

Files	Patch %	Lines
src/consts.rs	55.97%	70 Missing ⚠️
src/carrier.rs	0.00%	1 Missing ⚠️
src/error.rs	0.00%	1 Missing ⚠️
src/extension.rs	0.00%	1 Missing ⚠️
src/phone_number.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
- Coverage   71.39%   70.91%   -0.48%     
==========================================
  Files          19       20       +1     
  Lines        1930     2022      +92     
==========================================
+ Hits         1378     1434      +56     
- Misses        552      588      +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rubdos]

I generally agree with the modifications, but I'd like to discuss the MSRV policy before commiting to this.

[rubdos]

Is there a way to remain on 1.62? I don't necessarily know why we mandate regex = ~1.7, but I suppose there are a lot of other dependencies that will require 1.62+...

Reason I'd like to remain compatible with 1.62 is that Sailfish OS will have that as a compiler for the next few months.

If you don't feel like figuring it out, I can try to free up some time to traverse the dependency tree.

As a general remark: there is little reason to bump dependencies (like either) in Cargo.toml on feature level, if we don't require that feature level. The Cargo.toml should indicate the minimum required version of a crate.

[ds-cbo]

Hm, then you have an opposite look at Cargo.toml of what I do, I tend to keep dependencies at the latest possible version (given a MSRV) to get all the new bugfixes, performance improvements and features. For now I've downgraded them all to the lowest possible version given the MSRV of 1.62

[rubdos]

I tend to keep dependencies at the latest possible version (given a MSRV) to get all the new bugfixes, performance improvements and features.

That is generally the idea, but specifying regex = "1" means that by default, the latest in the 1-series will be used. There's no direct need to depend on 1.7 if we don't use 1.7 features.

[djc]

Friendly ping? Would be nice to get this released to avoid duplicate dependencies.

[ds-cbo]

@djc Is that a ping for me? I thought I processed all feedback and the ball is back at the maintainers now

[rubdos]

@djc Is that a ping for me? I thought I processed all feedback and the ball is back at the maintainers now

No it's to me, and I indeed forgot about this. Working on it!

[rubdos]

I took the liberty to push the dependency bumps on your branch here, in the way that I meant them. This version broadens the compatibility, without breaking any backwards compatibility:

• Users that cannot use regex 1.9 are not forced onto 1.9, but those that want to use it can (and already could) use it. regex = "1.7" signifies "we require regex 1, with a minimum of 1.7".
• The same for 0.x dependencies, although these require manual work. E.g. quick-xml = ">=0.28, <=0.31" means what it looks like it means. Before, quick-xml = "0.28" meant 0.28.x, because Rust interprets the 0 version a bit differently.

As far as I can see, all dependencies are hereby up-to-date.

The only part that required Rust 1.62 was the derive(Default). I took the liberty to revert that commit, because it is rather trivial and keeps us 1.58-compatible. If we ever bump above 1.62, we can of course put it back.

I'm having a look at the test-dependencies now, such that we can have MSRV tests in CI too.

[djc]

I'm having a look at the test-dependencies now, such that we can have MSRV tests in CI too.

I recommend testing MSRV in CI with cargo check --lib --all-features, that is, avoiding tests and dev-dependencies for the purpose of MSRV testing.

[rubdos]

I recommend testing MSRV in CI with cargo check --lib --all-features, that is, avoiding tests and dev-dependencies for the purpose of MSRV testing.

Yes, looks like that's the way to go. I'm splitting this PR in two, one for the deps (#65) and one upcoming for the Lazy once_cell refactoring. The current state is a bit too much intertwined to decently review.

[djc]

FWIW, unless you put in the work to have CI jobs for each semver-incompatible branch (or have CI testing minimal-versions), I'm weary of >=0.10, <=0.12 style dependencies. I think this means Cargo will generally select 0.12.x in CI, so you're not actually testing whether 0.10 and 0.11 still work over time.

(Also, while I consider myself fairly conservative on MSRV issues, I think 1.6x is a pretty reasonable target at this point, especially for low x.)

[rubdos]

FWIW, unless you put in the work to have CI jobs for each semver-incompatible branch (or have CI testing minimal-versions), I'm weary of >=0.10, <=0.12 style dependencies. I think this means Cargo will generally select 0.12.x in CI, so you're not actually testing whether 0.10 and 0.11 still work over time.

I just tested, and apparently since the rust-version statements appeared in Cargo.toml, cargo is smart enough to check out the lower versions when necessary for lower rustc. Your point still stands though, there's no formal test around. Are you aware of any formal tests for these situations?

(Also, while I consider myself fairly conservative on MSRV issues, I think 1.6x is a pretty reasonable target at this point, especially for low x.)

I would still consider 1.61 at this point, since that's what SailfishOS will soon be on (if not 1.72). More out of personal "greed" or interest than anything else, for which I'm sorry.

[rubdos]

I'll close this in favor of #65 and #66.