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.

Sign up for GitHub

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

Jump to bottom

rustdoc: Turn JsonRenderer::mod_item_in into unreachable!() #84459

Merged
merged 1 commit into from
Apr 24, 2021
Merged

rustdoc: Turn JsonRenderer::mod_item_in into unreachable!() #84459

merged 1 commit into from
Apr 24, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 22, 2021

The JSON renderer no longer gets called on modules (since #83055).

r? @aDotInTheVoid

@jyn514 jyn514 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. A-rustdoc-json Area: Rustdoc JSON backend labels Apr 22, 2021
aDotInTheVoid
aDotInTheVoid reviewed Apr 23, 2021
View reviewed changes
src/librustdoc/json/mod.rs Outdated Show resolved Hide resolved
src/librustdoc/json/mod.rs Outdated
self.item(item.clone()).unwrap();
Ok(())
fn mod_item_in(&mut self, _item: &clean::Item, _module_name: &str) -> Result<(), Error> {
unreachable!()
}

fn mod_item_out(&mut self, _item_name: &str) -> Result<(), Error> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: This can also be make unreachable!

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh hmm, I think I was assuming this already had the changes from https://github.com/rust-lang/rust/pull/84458/files#diff-46f011cf734f3de35eddca150990366cc41bd8eaf3945a241b2a2f79c5bb77eaR40. Do you think it's important to add the assert given that this function gets removed altogether there? Or is it fine to have it silently do nothing? If the latter I'd prefer to leave this as-is to avoid merge conflicts.

Copy link
Member

Choose a reason for hiding this comment

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

I think its fine to leave then, as it would be very wierd to call mod_item_out without mod_item_in

@aDotInTheVoid
Copy link
Member

2 Minor nits, also I cant approve because I don't have r+ rights.

@aDotInTheVoid aDotInTheVoid mentioned this pull request Apr 23, 2021
Open
@jyn514 jyn514 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 23, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 23, 2021

@bors delegate=aDotInTheVoid

@bors
Copy link
Contributor

bors commented Apr 23, 2021

✌️ @aDotInTheVoid can now approve this pull request

@aDotInTheVoid
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 23, 2021

📌 Commit 315bfdb84868f2e72f1e13a44ed8f5a9ba367c82 has been approved by aDotInTheVoid

@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 Apr 23, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 23, 2021

Thanks for the review!

@bors

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2021
@jyn514
rustdoc: Turn JsonRenderer::mod_item_in into unreachable!()
03ace76 
The JSON renderer no longer gets called on modules.
@jyn514
Copy link
Member Author

jyn514 commented Apr 24, 2021

@bors r=aDotInTheVoid

@bors
Copy link
Contributor

bors commented Apr 24, 2021

📌 Commit 03ace76 has been approved by aDotInTheVoid

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Apr 24, 2021
@JohnTitor
Rollup merge of rust-lang#84459 - jyn514:json-mod, r=aDotInTheVoid
eb727a1 
rustdoc: Turn `JsonRenderer::mod_item_in` into `unreachable!()`

The JSON renderer no longer gets called on modules (since rust-lang#83055).

r? `@aDotInTheVoid`
This was referenced Apr 24, 2021
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 24, 2021
@bors
Auto merge of rust-lang#84525 - JohnTitor:rollup-t2qigt3, r=JohnTitor
7e11f3a 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#83519 (Implement a lint that highlights all moves larger than a configured limit)
 - rust-lang#84105 (stabilize `core::array::{from_ref,from_mut}` in `1.53.0`)
 - rust-lang#84179 (Explicitly implement `!Send` and `!Sync` for `sys::{Args, Env}`)
 - rust-lang#84427 (Update Clippy)
 - rust-lang#84459 (rustdoc: Turn `JsonRenderer::mod_item_in` into `unreachable!()`)
 - rust-lang#84460 (rustdoc: Remove unnecessary `is_crate` field from doctree::Module and clean::Module)
 - rust-lang#84464 (rustdoc: Get rid of `clean::TypeKind`)
 - rust-lang#84518 (Clean up DOM strings)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 46c517b into rust-lang:master Apr 24, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 24, 2021
@jyn514 jyn514 deleted the json-mod branch April 24, 2021 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aDotInTheVoid aDotInTheVoid aDotInTheVoid left review comments

Assignees
No one assigned
Labels
A-rustdoc-json Area: Rustdoc JSON backend 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.53.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jyn514 @aDotInTheVoid @bors @rustbot