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

Use macros for module declarations on registry #1074

Merged
merged 3 commits into from
Nov 9, 2020

Conversation

k-nasa
Copy link
Contributor

@k-nasa k-nasa commented Oct 31, 2020

Motivation

ref: #1009

This is code refactoring. Currently, there is a lot of code that declares modules as follows:

#[cfg(feature = "registry")]
#[cfg_attr(docsrs, doc(cfg(feature = "registry")))]
pub use registry::Registry;

#[cfg(feature = "registry")]
#[cfg_attr(docsrs, doc(cfg(feature = "registry")))]
pub fn registry() -> Registry {
    Registry::default()
}

It is verbose to write a module declaration multiple times using the feature attribute. Also, if the definition for the same feature spans multiple places, it will be a little harder to read the code.

Solution

You can combine features attributes in one place by writing as follows. It also eliminates the need to write the same code over and over again.

cfg_feature!("feature" ,{
    pub use registry::Registry;

    pub fn registry() -> Registry {
        Registry::default()
    }
});

@k-nasa k-nasa requested review from hawkw and a team as code owners October 31, 2020 08:15
@k-nasa k-nasa changed the title Use macros for module declarations Use macros for module declarations on registry Oct 31, 2020
@davidbarsky
Copy link
Member

Thanks! Can you format this PR to pass CI?

@k-nasa
Copy link
Contributor Author

k-nasa commented Nov 3, 2020

I fix it!

@davidbarsky
Copy link
Member

Thank you, and sorry for the delay! I was on vacation last week.

@davidbarsky davidbarsky merged commit 710228e into tokio-rs:master Nov 9, 2020
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