fix(storage): StorageImpl nameable outside crate#3683
fix(storage): StorageImpl nameable outside crate#3683dbolduc merged 2 commits intogoogleapis:mainfrom
StorageImpl nameable outside crate#3683Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3683 +/- ##
==========================================
+ Coverage 95.75% 95.93% +0.18%
==========================================
Files 129 131 +2
Lines 5157 5197 +40
==========================================
+ Hits 4938 4986 +48
+ Misses 219 211 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/storage/src/storage/client.rs
Outdated
| /// [Application Default Credentials]: https://cloud.google.com/docs/authentication#adc | ||
| #[derive(Clone, Debug)] | ||
| pub struct Storage<S = crate::storage::transport::Storage> | ||
| pub struct Storage<S = crate::stub::StorageImpl> |
There was a problem hiding this comment.
Maybe crate::stub::DefaultStorageStub ?
There was a problem hiding this comment.
Thanks for the suggestion. I will go with crate::stub::DefaultStorage to not repeat "stub"
src/storage/src/lib.rs
Outdated
| pub mod stub { | ||
| pub use crate::control::stub::*; | ||
| pub use crate::storage::stub::*; | ||
| pub use crate::storage::transport::Storage as StorageImpl; |
There was a problem hiding this comment.
This makes the type public, which means it goes into the documentation, and it current has no documentation. Luckily the constructor is pub(crate), so at least there are no new methods to document.
There was a problem hiding this comment.
and it currently has no documentation
This PR adds documentation.
There was a problem hiding this comment.
You help me realize that the pub mod stub did not have documentation. Sent #3690 for that.
src/storage/src/storage.rs
Outdated
| pub mod streaming_source; | ||
| pub mod stub; | ||
| pub(crate) mod transport; | ||
| pub mod transport; |
There was a problem hiding this comment.
Do we need to make this change? It makes the module public and it would be only too easy to add a public symbol by accident in that module too?
There was a problem hiding this comment.
Do we need to make this change?
Apparently not. Thanks for catching. Reverted.
It makes the module public
I didn't think so. It is within the non-public mod storage:
google-cloud-rust/src/storage/src/lib.rs
Line 43 in b0dcf16
it would be only too easy to add a public symbol by accident in that module too?
We would have to accidentally export it too.
Fixes #3663
Make the default stub implementation name-able from outside the crate. This lets applications provide a default implementation for their own generics.
Note that this type is not constructible from outside the crate. It has no public fields. Its only public fns are its impl of
stub::Storage.I think this type belongs in the
stubmodule. I called itStorageImpl.If we are happy with these names, I will clean up the internal wiring, either in this PR or in a follow up.