rustdoc: Remove apparently unnecessary conditional in `doc_value` by camelid · Pull Request #91305 · rust-lang/rust

camelid · 2021-11-28T01:16:18Z

I need to remove this conditional for #91072, but while it seems
unnecessary, we are not certain. So, the plan is to first remove the
conditional and see if any regressions pop up before doing the refactor.
This way, it will be easier to revert if there are subtle regressions.

r? @jyn514

camelid · 2021-11-28T01:19:54Z

Note for the future: This PR is designed to be easily reverted. Feel free to revert it if significant regressions arise.

rustdoc: Write doc-comments directly instead of using FromIterator The FromIterator impl made the code much harder to understand. The types don't make sense until you realize there's a custom FromIterator impl. This is the first commit from rust-lang#91305; since `@camelid` wrote it originally I don't feel bad unilaterally approving it. r? `@ghost` `@bors` r+ Note that this will conflict with rust-lang#92078.

rustdoc: Write doc-comments directly instead of using FromIterator The FromIterator impl made the code much harder to understand. The types don't make sense until you realize there's a custom FromIterator impl. This is the first commit from rust-lang#91305; since ``@camelid`` wrote it originally I don't feel bad unilaterally approving it. r? ``@ghost`` ``@bors`` r+ Note that this will conflict with rust-lang#92078.

…eGomez rustdoc: Remove unused `collapsed` field `render/context` always runs after `run_global_context`, so it was always set to `true`. This is a holdover from when rustdoc allowed configuring passes, but the `collapse-docs` pass was removed ages ago, and the ability to configure passes is about to be removed. Found while reviewing rust-lang#91305.

I need to remove this conditional for rust-lang#91072, but while it seems unnecessary, we are not certain. So, the plan is to first remove the conditional and see if any regressions pop up before doing the refactor. This way, it will be easier to revert if there are subtle regressions.

camelid · 2021-12-22T02:01:22Z

@rustbot ready

jyn514 · 2022-01-04T22:59:03Z

I still am very uncomfortable making a change we know changes the behavior, but I don't want to block it.

r? @GuillaumeGomez

Mark-Simulacrum · 2022-01-04T23:02:40Z

 for new_frag in iter {
- if new_frag.kind != ori.kind || new_frag.parent_module != ori.parent_module {
- break;
- }


nit: this means that the iter.next() and such seems no longer necessary here (all elements of the iterator are directly added), but if you're just leaving that for easier revert seems OK.

Let's see what @camelid thinks about it.

That's a good point, but I do want to make this easy to revert quickly. So I think let's leave it as-is for now.

GuillaumeGomez

Apart from @Mark-Simulacrum's comment, looks good to me.

camelid · 2022-01-06T19:44:33Z

Hmm... GitHub didn't notify me until just now.

camelid · 2022-01-06T19:46:10Z

@bors r=GuillaumeGomez rollup=never (for easy bisecting)

bors · 2022-01-06T19:46:11Z

📌 Commit 82d8ed4 has been approved by GuillaumeGomez

bors · 2022-01-07T05:39:47Z

⌛ Testing commit 82d8ed4 with merge 6702eb6519fd3567230d593f986503fd38329651...

bors · 2022-01-07T08:47:22Z

💔 Test failed - checks-actions

rust-log-analyzer · 2022-01-07T08:47:39Z

A job failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

GuillaumeGomez · 2022-01-07T10:05:06Z

@bors retry

bors · 2022-01-07T12:04:47Z

⌛ Testing commit 82d8ed4 with merge 7d60dbbf68500d22760ab06370681c36880cd021...

bors · 2022-01-07T16:19:47Z

💥 Test timed out

rust-log-analyzer · 2022-01-07T16:52:21Z

A job failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

ehuss · 2022-01-07T17:13:20Z

@bors retry

x86_64-msvc-1 took more than 4.5 hours to finish

bors · 2022-01-08T08:55:48Z

⌛ Testing commit 82d8ed4 with merge 0282233...

bors · 2022-01-08T12:06:33Z

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 0282233 to master...

rust-timer · 2022-01-08T13:37:09Z

Finished benchmarking commit (0282233): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

rustdoc: Remove unused `collapsed` field `render/context` always runs after `run_global_context`, so it was always set to `true`. This is a holdover from when rustdoc allowed configuring passes, but the `collapse-docs` pass was removed ages ago, and the ability to configure passes is about to be removed. Found while reviewing rust-lang/rust#91305.

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 28, 2021

rust-highfive assigned jyn514 Nov 28, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2021

camelid mentioned this pull request Nov 28, 2021
rustdoc: Remove distinction between "regular" and "collapsed" docs #91072
Closed

This was referenced Dec 18, 2021
rustdoc: Remove unused collapsed field #92077
Merged
rustdoc: Write doc-comments directly instead of using FromIterator #92082
Merged

This comment has been minimized.
Sign in to view

camelid 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 22, 2021

camelid force-pushed the rm-cond branch from eb257cc to 82d8ed4Compare December 22, 2021 02:01

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 22, 2021

rust-highfive assigned GuillaumeGomez and unassigned jyn514 Jan 4, 2022

Mark-Simulacrum reviewed Jan 4, 2022
View reviewed changes

GuillaumeGomez approved these changes Jan 6, 2022
View reviewed changes

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 Jan 6, 2022

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 7, 2022

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 Jan 7, 2022

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 7, 2022

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 Jan 7, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 8, 2022

bors merged commit 0282233 into rust-lang:masterJan 8, 2022

rustbot added this to the 1.59.0 milestone Jan 8, 2022

camelid deleted the rm-cond branch January 8, 2022 19:37

Uh oh!

Uh oh!

Conversation

camelid commented Nov 28, 2021

Uh oh!

camelid commented Nov 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

camelid commented Dec 22, 2021

Uh oh!

jyn514 commented Jan 4, 2022

Uh oh!

Mark-SimulacrumJan 4, 2022

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomezJan 6, 2022

Choose a reason for hiding this comment

Uh oh!

camelidJan 6, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez left a comment

Choose a reason for hiding this comment

Uh oh!

camelid commented Jan 6, 2022

Uh oh!

camelid commented Jan 6, 2022

Uh oh!

bors commented Jan 6, 2022

Uh oh!

bors commented Jan 7, 2022

Uh oh!

bors commented Jan 7, 2022

Uh oh!

rust-log-analyzer commented Jan 7, 2022

Uh oh!

GuillaumeGomez commented Jan 7, 2022

Uh oh!

bors commented Jan 7, 2022

Uh oh!

bors commented Jan 7, 2022

Uh oh!

rust-log-analyzer commented Jan 7, 2022

Uh oh!

ehuss commented Jan 7, 2022

Uh oh!

bors commented Jan 8, 2022

Uh oh!

bors commented Jan 8, 2022

Uh oh!

rust-timer commented Jan 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

camelid commented Nov 28, 2021•
edited
Loading

camelidJan 6, 2022•
edited
Loading