Deprecate legacy items #1141
Deprecate legacy items #1141
Conversation
github-actions
bot
added
the
semver-checks-breaking
|
See the following report for details: cargo semver-checks output |
|
semver-checks deem this PR API-breaking, but if you read its message through, you'll see that it only requires a minor version bump, not major - so it's OK to do the changes in 0.15.1 (because we consider the last number the minor version number). |
We should make message and tag different if only a minor version bump is required. Especially after 1.0 |
|
Please don't merge yet. I'll update thiserror after 2.0.6 got released, so that there are no unnecessary module-level `#![allow(deprecated)] attributes. |
wprzytula
force-pushed
the
deprecate-legacy-items
branch
from
57072e7 to
6bab921
Compare
There was a problem hiding this comment.
Commit: "codewide: deprecate legacy serialization API's items "
Do we strictly need this "legacy" module? Why not just deprecate the items?
If it is required for some reason, then it's creation should be a separate commit, not the same one that adds deprecation.
thiserror fixed handling of deprecated items in 2.0.6, which is crucial for the next commits introducing deprecation notice for legacy types. Note: this is not a breaking change, because thiserror does not appear in the public API.
There were some remnants from the legacy deserialization API that had not been marked with #[deprecated] attribute.
There are a huge lot of items, it would be a lot of meaningless work to attribute each of them. |
But you are also applying deprecation notices to the items inside this new module, not only to the module itself. |
To make deprecations in value.rs less invasive, as measured by the number of required changes in the next commit, a `legacy` module is extracted for all legacy items. As a bonus, the future deletion of legacy items will be clearer. I recommend viewing this commit with whitespace changes ignored.
wprzytula
force-pushed
the
deprecate-legacy-items
branch
from
6bab921 to
da6102a
Compare
Items constituting the legacy serialization API hadn't been marked with the #[deprecated] attribute. As we're going to remove them soon, let's warn users about that explicitly.
wprzytula
force-pushed
the
deprecate-legacy-items
branch
from
da6102a to
c136255
Compare
Those notices from inside the new module were leftovers. Now there is per-module global deprecation notice. |
| #[derive(Debug, Error, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] | ||
| pub enum SerializeValuesError { | ||
| #[error("Too many values to add, max 65,535 values can be sent in a request")] | ||
| TooManyValues, | ||
| #[error("Mixing named and not named values is not allowed")] | ||
| MixingNamedAndNotNamedValues, | ||
| #[error(transparent)] | ||
| ValueTooBig(#[from] ValueTooBig), | ||
| #[error("Parsing serialized values failed")] | ||
| ParseError, | ||
| } | ||
|
|
There was a problem hiding this comment.
Why is this not part of the legacy module?
There was a problem hiding this comment.
SerializeValuesError is used in types::serialize::row::SerializedValues::add_value, and SerializeValuesError contains a variant for ValueTooBig. That's why I left those not deprecated.
Deprecate legacy items (cherry picked from commit 868d86f)
The following items are now marked as deprecated:
Serialization:
scylla::impl_serialize_value_via_valuescylla::impl_serialize_row_via_value_listscylla::ValueListscylla::serialize::batch::LegacyBatchValuesAdapterscylla::serialize::batch::LegacyBatchValuesIteratorAdapterscylla::serialize::row::ValueListAdapterscylla::serialize::row::ValueListToSerializeRowAdapterErrorscylla::serialize::row::serialize_legacy_rowscylla::serialize::value::ValueAdapterscylla::serialize::value::ValueToSerializeValueAdapterErrorscylla::serialize::value::serialize_legacy_valuescylla_cql::frame::value::LegacyBatchValuesscylla_cql::frame::value::LegacyBatchValuesFirstSerializedscylla_cql::frame::value::LegacyBatchValuesIteratorscylla_cql::frame::value::LegacyBatchValuesFromIterscylla_cql::frame::value::LegacyBatchValuesIteratorFromIteratorscylla_cql::frame::value::TupleValuesIterscylla_cql::frame::value::ValueDeserialization:
scylla::impl_from_cql_value_from_methodscylla::transport::iterator::LegacyNextRowErrorscylla::transport::legacy_query_result::IntoLegacyQueryResultErrorscylla::transport::legacy_query_result::MaybeFirstRowTypedErrorscylla::transport::legacy_query_result::RowsExpectedErrorscylla::transport::legacy_query_result::SingleRowErrorscylla::transport::legacy_query_result::IntoTypedRowsscylla::transport::legacy_query_result::TypedRowIterscylla_cql::cql_to_rust::FromCqlValErrorscylla_cql::cql_to_rust::FromRowErrorFixes: #1139
Pre-review checklist
[ ] I added relevant tests for new features and bug fixes.[ ] I have provided docstrings for the public items that I want to introduce.[ ] I have adjusted the documentation in./docs/source/.Fixes:annotations to PR description.