Decrease pub visibility of scylla-cql definitions
#933
Conversation
muzarski
force-pushed
the
decrease-pub-usage
branch
from
e2124ad
to
e6c4b44
Compare
There was a problem hiding this comment.
Looks like a careful step in the right direction.
This PR only hides some definitions, without modyfing any import paths - which is good from the aspect of not breaking existing code.
For comparison, please see my approach to module refactor, which additionally restructures import paths in a way that makes more sense for me:
https://github.com/wprzytula/scylla-rust-driver/blob/restructure-modules/scylla/src/cql/mod.rs
wprzytula
added
the
waiting-on-author
Lorak-mmk
removed
the
waiting-on-author
|
@muzarski we can merge after you rebase on main (there is a conflict) |
Pub re-exported the structs of scylla_cql::frame module and its submodules that contain any of the symbols that should be exposed publicly. This means that the following submodules will be private: - scylla_cql::frame::request - scylla_cql::frame::server_event_type - scylla_cql::frame::protocol_features
Re-exported the Consistency and SerialConsistency types. Hidden utility functions such as `read/write_int`.
pub re-exported submodules that contain publicly used types. The submodules re-exported as pub: - scylla_cql::frame::response::cql_to_rust - scylla_cql::frame::response::result The submodules re-exported as pub(crate): - scylla_cql::frame::response::authenticate - scylla_cql::frame::response::error - scylla_cql::frame::response::event - scylla_cql::frame::response::supported
Re-exported types that should be exposed to users. Re-exported types: - ColumnSpec - ColumnType - CqlValue - PartitionKeyIndex - Row - TableSpec Note for reviewers: As mentioned in scylladb#925 (comment), we should not expose the `PreparedMetadata` structure. I believe the same applies to `ResultMetadata`. That's why I decided not to re-export them.
muzarski
force-pushed
the
decrease-pub-usage
branch
from
e6c4b44
to
73bbf87
Compare
|
v2: resolved conflicts with main |
|
|
| pub(crate) use scylla_cql::frame::response::*; | ||
|
|
||
| pub mod result { | ||
| pub(crate) use scylla_cql::frame::response::result::*; |
There was a problem hiding this comment.
I'm not sure if I'm happy with the glob imports here. Being pub(crate) ones, they are relatively benign though.
Motivation
Some definitions in
scylla-cqlcrate need to be public, so thescyllacrate can make use of them. However, currently all of the definitions are implicitly re-exported outside and can be seen by the driver users. The idea is to explicitly re-export the types/functions that should be visible outside (and leave the rest of the public definitions for the internal usage ofscyllacrate).re-exported types
The main difficulty was to decide on what types should be re-exported publicly. The types that got re-exported can be seen in the documentation (
cargo doc --open).Error types
I wasn't sure whether error types such as
scylla_cql::frame::frame_errors::FrameErrorshould be public. As of now, they are left public but this can be further discussed.Pre-review checklist
./docs/source/.Fixes:annotations to PR description.