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

moved all code into sealed:: mod to make API tagging possible #987

Closed
wants to merge 2 commits into from

Conversation

milyin
Copy link
Contributor

@milyin milyin commented Apr 27, 2024

The whole zenoh library API is present in lib.rs file with "pub use" reexports. No file mods are public.

This allows to overview all the API from single file. But this works one direction only. I.e. structure/trait/function belongs to API if it's declared in public module in lib.rs. But it's impossible to say is some structure/function/method public by just looking at it's definition itself. Even if it's pub(crate) it still can be reexported by lib.rs.

This may cause inconveniences when reading code. Also, which is more important, this breaks logic of tagging tool https://github.com/zettaScaleLabs/api-matrix. It needs explicit pub declarations for interface items, so the fact that pub(crate) instances still can be part of interface makes it's diagnostics incorrect.

This update removes pub(crate) statements replacing them to pub(in crate::sealed). Unlike pub(crate) the pub(in crate::sealed) can't be reexported because the lib.rs don't have access to them. This forces us to explicitly mark all interface elements as pub. I.e. if something belongs to API it should be both reexported in lib.rs and marked as pub in the source.

The usage of pub(crate) should be avoided as this doesn't explicitly state if item belongs to API or not.

@milyin milyin requested review from Mallets and OlivierHecart April 28, 2024 18:20
@@ -36,7 +36,7 @@
//! ```
// Reexport API in flat namespace
pub(crate) mod flat {
pub mod flat {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here pub(crate) (or better pub(self)?) should be restored: these namespaces are technical, they are not part of public API

@milyin
Copy link
Contributor Author

milyin commented Apr 29, 2024

Joseph — Today at 10:43 AM
I've tried to replace pub KeyExpr by pub(crate) KeyExpr, and I got
error[E0364]: KeyExpr is only public within the crate, and cannot be re-exported outside

So I admit I don't understand the issue you try to solve
See https://doc.rust-lang.org/error_codes/E0364.html

Michael Ilyin — Today at 11:27 AM
Yes, you are right. I can publicly reexport from "pub(crate)" mods but not the "pub(crate)" instances itself. Then yes, the problem doesn't exists, the PR is unnecessary

@milyin milyin closed this Apr 29, 2024
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

Successfully merging this pull request may close these issues.

1 participant