8000 Always display stability version even if it's the same as the containing item by GuillaumeGomez · Pull Request #118441 · rust-lang/rust · GitHub
[go: up one dir, main page]

Skip to content

Conversation

GuillaumeGomez
Copy link
Member

Fixes #118439.

Currently, if the containing item's version is the same as the item's version (like a method), we don't display it on the item.

This was something done on purpose as you can see here. It was implemented in #30686.

I think we should change this because on pages with a lot of items, if someone arrives (through the search or a link) to an item far below the page, they won't know the stability version unless they scroll to the top, which isn't great.

You can see the result here.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 29, 2023
@rustbot
Copy link
Collaborator
rustbot commented Nov 29, 2023

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

@notriddle
Copy link
Contributor

This is definitely going to make the standard library docs bigger. Let's see how much!

@bors try
@rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 29, 2023
@bors
Copy link
Collaborator
bors commented Nov 29, 2023

⌛ Trying commit a333572 with merge 4d962f0...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2023
…sion, r=<try>

Always display stability version even if it's the same as the containing item

Fixes rust-lang#118439.

Currently, if the containing item's version is the same as the item's version (like a method), we don't display it on the item.

This was something done on purpose as you can see [here](https://github.com/rust-lang/rust/blob/e9b7bf011478aa8c19ac49afc99853a66ba04319/src/librustdoc/html/render/mod.rs#L949-L955). It was implemented in rust-lang#30686.

I think we should change this because on pages with a lot of items, if someone arrives (through the search or a link) to an item far below the page, they won't know the stability version unless they scroll to the top, which isn't great.

You can see the result [here](https://rustdoc.crud.net/imperio/display-stability-version/std/pin/struct.Pin.html#method.new).

r? `@notriddle`
@bors
Copy link
Collaborator
bors commented Nov 29, 2023

☀️ Try build successful - checks-actions
Build commit: 4d962f0 (4d962f0f089466aec16928808dde2312d85fe7c1)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4d962f0): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.6% [-4.7%, -4.4%] 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 673.033s -> 673.683s (0.10%)
Artifact size: 313.38 MiB -> 313.38 MiB (-0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 29, 2023
@Kobzol
Copy link
Member
Kobzol commented Nov 29, 2023

Hmm. The sizes are the same (https://perf.rust-lang.org/compare.html?start=abe34e9ab14c0a194152b4f9acc3dcbb000f3e98&end=4d962f0f089466aec16928808dde2312d85fe7c1&stat=size%3Adoc_bytes&tab=compile&nonRelevant=true&showRawData=true). But I guess that's expected, since non-std documentation doesn't actually contain these attributes (?). We should probably measure the size of libstd docs, but that's not included in the benchmarks at the moment.

@notriddle
Copy link
Contributor

Yeah. I didn't think of that. Here's the output for ./x doc library/std, run on my machine:

michaelhowell@Michael-Howells-Macbook-Pro rust % du -hs doc-new doc-old
117M    doc-new
116M    doc-old
michaelhowell@Michael-Howells-Macbook-Pro rust % du -s doc-new doc-old 
240272  doc-new
237384  doc-old

Where doc-new is the version in this branch (a333572), and doc-old is from b29a1e0

@Kobzol
Copy link
Member
Kobzol commented Nov 29, 2023

So about a 1.2% increase. That's not that bad I guess, but it's also not trivially small.

@notriddle
Copy link
Contributor
notriddle commented Nov 29, 2023

@rfcbot fcp close

I have to say it's not worth it. There's reasonable arguments for both perspectives, and I don't want to be flip-flopping back and forth.

@rfcbot
Copy link
rfcbot commented Nov 29, 2023

Team member @notriddle has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.