Skip to content

rustdoc: Rename Type::ResolvedPath to Type::Path and don't re-export it #91197

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Nov 27, 2021
Merged

rustdoc: Rename Type::ResolvedPath to Type::Path and don't re-export it #91197

merged 3 commits into from
Nov 27, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Nov 24, 2021 •
edited
Loading

The new name is shorter, simpler, and consistent with hir::Ty. It can't be
re-exported since the name would conflict with the clean::Path struct. But
usually enum variants are referred to using their qualified names in Rust anyway
(and parts of rustdoc already do that with clean::Type), so this is also more
consistent with the language.

r? @GuillaumeGomez
cc @jyn514

@camelid camelid added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 24, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2021
@camelid camelid added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 24, 2021
@GuillaumeGomez
Copy link
Member

I'm afraid that the new name is too generic: we already have clean::Path, ast::Path and hir::Path. But as you mentioned, the biggest issue here is clean::Path. I'm really not sure if it's a good idea...

@camelid
Copy link
Member Author

camelid commented Nov 24, 2021

I'm afraid that the new name is too generic: we already have clean::Path, ast::Path and hir::Path. But as you mentioned, the biggest issue here is clean::Path. I'm really not sure if it's a good idea...

It's the same name as hir::TyKind::Path, and it's just a wrapper for clean::Path. Also, I don't see why ResolvedPath is any less generic; all paths are resolved.

@GuillaumeGomez
Copy link
Member

It's different, not less generic. That was the main point. Well, maybe I'm the only who thinks that. What do you think @rust-lang/rustdoc ?

@jyn514
Copy link
Member

jyn514 commented Nov 24, 2021

@camelid @GuillaumeGomez can you please use Zulip threads instead of pings for low-impact things like this? I get a lot of pings and this isn't a user-facing change.

@jyn514
Copy link
Member

jyn514 commented Nov 24, 2021

Anyway, I think this isn't worth worrying about until #91195 is merged.

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Nov 25, 2021
@camelid
Clean up clean re-exports
8adb0b6 
This will allow re-exporting only certain enum variants.
@camelid
Stop re-exporting Type::ResolvedPath
d81deb3 
I would like to rename it to `Type::Path`, but then it can't be
re-exported since the name would conflict with the `Path` struct.
Usually enum variants are referred to using their qualified names in
Rust (and parts of rustdoc already do that with `clean::Type`), so this
is also more consistent with the language.
@camelid camelid force-pushed the rename-resolvedpath branch from c155fa7 to 9a09e07 Compare November 25, 2021 20:08
@camelid
Copy link
Member Author

camelid commented Nov 25, 2021

Anyway, I think this isn't worth worrying about until #91195 is merged.

It's merged now ;)

@jyn514
Copy link
Member

jyn514 commented Nov 25, 2021

I don't think it's a bad idea given that it's only a variant and directly contains a clean::Path. I don't feel strongly either way though.

@GuillaumeGomez
Copy link
Member

I don't have a strong opinion either. Well, I guess usage will tell. Unless you have something to do in this PR, r=me,jyn514.

@camelid
Copy link
Member Author

camelid commented Nov 26, 2021

@bors r=GuillaumeGomez,jyn514

@bors
Copy link
Collaborator

bors commented Nov 26, 2021

📌 Commit 9a09e07a55306ee4096d110af3c3395a3505dd9f has been approved by GuillaumeGomez,jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 26, 2021
@camelid
Rename Type::ResolvedPath to Type::Path
79c718f 
At last! The new name is shorter, simpler, and consistent with
`hir::Ty`.
@camelid camelid force-pushed the rename-resolvedpath branch from 9a09e07 to 79c718f Compare November 26, 2021 20:41
@camelid
Copy link
Member Author

camelid commented Nov 26, 2021

Updated references to ResolvedPath.

@bors r=GuillaumeGomez,jyn514

@bors
Copy link
Collaborator

bors commented Nov 26, 2021

📌 Commit 79c718f has been approved by GuillaumeGomez,jyn514

@matthiaskrgr matthiaskrgr mentioned this pull request Nov 26, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2021
@bors
Auto merge of rust-lang#91269 - matthiaskrgr:rollup-jh8i8eh, r=matthi…
bbad745 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90611 (Fix another ICE in rustdoc scrape_examples)
 - rust-lang#91197 (rustdoc: Rename `Type::ResolvedPath` to `Type::Path` and don't re-export it)
 - rust-lang#91223 (Fix headings indent)
 - rust-lang#91240 (Saner formatting for UTF8_CHAR_WIDTH table)
 - rust-lang#91248 (Bump compiler-builtins to 0.1.53)
 - rust-lang#91252 (Fix bug where submodules wouldn't be updated when running x.py from a subdirectory)
 - rust-lang#91259 (Remove `--display-doctest-warnings`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fcbbdaf into rust-lang:master Nov 27, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 27, 2021
@camelid camelid deleted the rename-resolvedpath branch November 27, 2021 18:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@GuillaumeGomez GuillaumeGomez

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.58.0
Development

Successfully merging this pull request may close these issues.
6 participants
@camelid @GuillaumeGomez @jyn514 @bors @rust-highfive @rustbot