-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Remove crate_visibility_modifier
feature
#1143
Conversation
In some situations, `pub(crate)` wasn't strictly necessary, as it was within a context where it would not be exposed regardless of scope. In these situations, `pub` is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do a more thorough review against a diff, but generally LGTM.
For reproducing my check, you'll want to generate docs for the current master and this PR. Move the In both of these directories, run Next, run As of now, there are no lines with differing lengths, indicating that nothing was exposed or hidden accidentally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks to be either crate -> pub(crate)
or a crate -> pub
that I have checked by hand to be contained in a non-pub
path.
Test failures look unrelated -- UI test regression possibly from rust-lang/rust#64498. |
@jebrosen That looks like a serious regression. I'm not sure if that's the PR to blame, but we should figure out what is. |
You would actually need to do this with all features enabled in both debug and release mode. But that difference only applies to one or two types, and I also manually checked that everything is okay. I'll merge once my local test run finishes. |
Awesome! Merged as 4e6a7dd. |
In some situations,
pub(crate)
wasn't strictly necessary, as it waswithin a context where it would not be exposed regardless of scope. In
these situations,
pub
is used per request.Pinging @SergioBenitez and @jebrosen for review. This should be looked through carefully to ensure nothing is inadvertently exposed or hidden. Checking the documentation should be sufficient for this.