Uh oh!
There was an error while loading. Please reload this page.
Fix invalid missing documentation lint reporting for re-exports - #109176
Fix invalid missing documentation lint reporting for re-exports#109176GuillaumeGomez wants to merge 4 commits into
Conversation
rustbot
commented
Mar 15, 2023
(rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
rustbot
commented
Mar 15, 2023
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
Just discovered some weird re-exports when running in test mode in |
petrochenkov
commented
Mar 17, 2023
Is there any history behind these changes? What issues do they fix? Is this about documentation being lost on reexport items from other crates? |
GuillaumeGomez
commented
Mar 17, 2023
Sorry, I forgot to link the issue... I'll update the first comment too. The issue: #108570 |
petrochenkov
commented
Mar 17, 2023
I've just found #108570 too. So it's not about dropped docs, but about making the |
GuillaumeGomez
commented
Mar 17, 2023
If you have suggestions on how to prevent this duplication (which I'm not super happy about either...), I'd gladly hear it. 😉 |
petrochenkov
commented
Mar 17, 2023
In the meantime let's do a perf run. |
This comment has been minimized.
This comment has been minimized.
bors
commented
Mar 17, 2023
⌛ Trying commit a2ac0ff with merge e1a6761a2474c0d7d9183f20b15fa5426cc2c301... |
bors
commented
Mar 17, 2023
☀️ Try build successful - checks-actions |
1 similar comment
bors
commented
Mar 17, 2023
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Mar 18, 2023
Finished benchmarking commit (e1a6761a2474c0d7d9183f20b15fa5426cc2c301): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking 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. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Warning ⚠: The following benchmark(s) failed to build:
Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
|
GuillaumeGomez
commented
Mar 21, 2023
That's quite the regression. I need to check how to improve this situation. |
I suggest not spending time on attempts to improve the current approach. The |
GuillaumeGomez
commented
Mar 21, 2023
Sounds good to me, thanks! Closing then. |
petrochenkov
commented
Mar 21, 2023
@GuillaumeGomez
I thought I had some understanding, but then #107000 and related PRs changed the rules again. |
GuillaumeGomez
commented
Mar 21, 2023
It's not documented anywhere but it really should. I'll try to update the rustdoc book today so the rules of re-exports and |
Fixes#108570.
The problem was that, if an item is re-exported, we don't check if it's actually displayed into the documentation. To do so, I changed the check this way:
Like that, if an item is re-exported, it'll just be checked on the re-export directly.
Not sure who should be set as reviewer here so I'll just ping @notriddle and wait for bors to assign someone.