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

refactor(rpc): add rpc crate #375

Merged
merged 1 commit into from
Sep 18, 2023
Merged

refactor(rpc): add rpc crate #375

merged 1 commit into from
Sep 18, 2023

Conversation

dancoombs
Copy link
Collaborator

No description provided.

crates/rpc/Cargo.toml Outdated Show resolved Hide resolved
@@ -3,7 +3,7 @@ use std::time::{Duration, Instant};
use jsonrpsee::{helpers::MethodResponseResult, server::logger::Logger};

#[derive(Clone)]
pub struct RpcMetricsLogger;
pub(crate) struct RpcMetricsLogger;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain why you choose to use pub(crate) here? My understanding (correct me if I'm wrong) is that these wouldn't have been exposed outside the crate anyways since they're not visible in lib.rs, so is this being used as documentation? Should we have guidelines for when to use pub(crate)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is enforced by the unreachable_pub lint, which will reject this if its marked as pub but isn't exposed outside of the crate. I actually think this is a reasonable lint as this makes it very clear that this code is only used within the crate. That gives it slightly less of a priority when it comes to API design and documentation. Also, the interwebs is saying that if you use pub(crate) dead code will get marked and can be removed.

Base automatically changed from danc/builder-crate to main September 18, 2023 21:22
@dancoombs dancoombs merged commit 948a538 into main Sep 18, 2023
5 checks passed
@dancoombs dancoombs deleted the danc/rpc-crate branch September 18, 2023 21:32
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.

2 participants