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

Consider using cargo-semver-checks #172

Open
xd009642 opened this issue Jul 29, 2024 · 0 comments
Open

Consider using cargo-semver-checks #172

xd009642 opened this issue Jul 29, 2024 · 0 comments

Comments

@xd009642
Copy link

There was a semver breakage mentioned in #87 and I was sceptical that a builder pattern would actually solve the issue because it doesn't stop you changing APIs in a semver incompatible manner at all. So as I saw qdrant-client had been updated again since I last checked I ran a cargo-semver-checks on it and it failed. Between 1.10.1 and 1.10.2 there was a "technically" breaking change.

cargo semver-checks --baseline-version 1.10.1
     Parsing qdrant-client v1.10.2 (current)
      Parsed [  28.700s] (current)
     Parsing qdrant-client v1.10.1 (baseline)
      Parsed [  37.051s] (baseline)
    Checking qdrant-client v1.10.1 -> v1.10.2 (patch change)
     Checked [   0.193s] 76 checks: 75 pass, 1 fail, 0 warn, 0 skip

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.33.0/src/lints/module_missing.ron

Failed in:
  mod qdrant_client::qdrant::payloads, previously in file /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/qdrant-client-1.10.1/src/qdrant_client/builders/payloads.rs:1

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  66.212s] qdrant-client

So as it didn't list any items in the module it must be a public module with no public items was removed so the only code it can break is code doing: use qdrant_client::qdrant::payloads as payloads; This isn't a big breakage as a result and the lint is likely to become a warning instead of error lint in future. But I hope it goes some way to demonstrate the usefulness of cargo-semver checks.

We can also look at the 1.9.0 -> 1.10.2 change and see 3 major breakages:

cargo semver-checks --baseline-version 1.9.0
     Parsing qdrant-client v1.10.2 (current)
      Parsed [   4.369s] (current)
     Parsing qdrant-client v1.9.0 (baseline)
      Parsed [  36.069s] (baseline)
    Checking qdrant-client v1.9.0 -> v1.10.2 (minor change)
     Checked [   0.126s] 70 checks: 67 pass, 3 fail, 0 warn, 6 skip

--- failure constructible_struct_adds_field: externally-constructible struct adds field ---

Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.33.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field ScoredPoint.order_value in /home/daniel/rust/rust-client/src/qdrant.rs:4943
  field SparseVectorParams.modifier in /home/daniel/rust/rust-client/src/qdrant.rs:137
  field SparseIndexConfig.datatype in /home/daniel/rust/rust-client/src/qdrant.rs:287
  field VectorParams.multivector_config in /home/daniel/rust/rust-client/src/qdrant.rs:44
  field ShardTransferInfo.to_shard_id in /home/daniel/rust/rust-client/src/qdrant.rs:974
  field MoveShard.to_shard_id in /home/daniel/rust/rust-client/src/qdrant.rs:1017
  field Vector.vectors_count in /home/daniel/rust/rust-client/src/qdrant.rs:3079
  field RetrievedPoint.order_value in /home/daniel/rust/rust-client/src/qdrant.rs:5079
  field AbortShardTransfer.to_shard_id in /home/daniel/rust/rust-client/src/qdrant.rs:1069
  field RestartTransfer.to_shard_id in /home/daniel/rust/rust-client/src/qdrant.rs:1084

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.33.0/src/lints/enum_variant_added.ron

Failed in:
  variant ShardTransferMethod:ReshardingStreamRecords in /home/daniel/rust/rust-client/src/qdrant.rs:1635
  variant Datatype:Float16 in /home/daniel/rust/rust-client/src/qdrant.rs:1255
  variant ReplicaState:Resharding in /home/daniel/rust/rust-client/src/qdrant.rs:1591

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.33.0/src/lints/module_missing.ron

Failed in:
  mod qdrant_client::filters, previously in file /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/qdrant-client-1.9.0/src/filters.rs:1

     Summary semver requires new major version: 3 major and 0 minor checks failed
    Finished [  40.723s] qdrant-client
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

No branches or pull requests

1 participant