Skip to content

De-dup common code from ExternalCrate methods#142720

Merged
bors merged 1 commit intorust-lang:masterfrom
yotamofek:pr/rustdoc/ext-crate-cleanup
Jun 20, 2025
Merged

De-dup common code from ExternalCrate methods#142720
bors merged 1 commit intorust-lang:masterfrom
yotamofek:pr/rustdoc/ext-crate-cleanup

Conversation

@yotamofek
Copy link
Contributor

@yotamofek yotamofek commented Jun 19, 2025

Also, return an impl Iterator instead of collecting into a Vec. Not sure if that'll have a measurable perf impact, but I think this PR still cleans up the two methods it touches quite nicely.

(I'm having trouble finding a name for the common method I extracted, currently called foobar, would love suggestions!)

@rustbot
Copy link
Collaborator

rustbot commented Jun 19, 2025

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jun 19, 2025
yotamofek
yotamofek commented Jun 19, 2025
View reviewed changes
@yotamofek yotamofek force-pushed the pr/rustdoc/ext-crate-cleanup branch from 05b2698 to 203bbe1 Compare June 19, 2025 14:11
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 19, 2025

@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rust-bors
Copy link
Contributor

rust-bors bot commented Jun 19, 2025

⌛ Trying commit 203bbe1 with merge df0a357…

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jun 19, 2025
@rust-bors
Auto merge of #142720 - yotamofek:pr/rustdoc/ext-crate-cleanup, r=<try>
df0a357 
De-dup common code from `ExternalCrate` methods

<!-- homu-ignore:start -->
<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r? <reviewer name>
-->
<!-- homu-ignore:end -->
Also, return an `impl Iterator` instead of collecting into a `Vec`. Not sure if that'll have a measurable perf impact, but I think this PR still cleans up the two methods it touches quite nicely.

(I'm having trouble finding a name for the common method I extracted, currently called `foobar`, would love suggestions!)
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 19, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@yotamofek yotamofek force-pushed the pr/rustdoc/ext-crate-cleanup branch from 203bbe1 to aca0688 Compare June 19, 2025 14:25
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 19, 2025

@bors2 try cancel

@rust-bors
Copy link
Contributor

rust-bors bot commented Jun 19, 2025

Try build cancelled. Cancelled workflows:

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 19, 2025

@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
rust-bors bot added a commit that referenced this pull request Jun 19, 2025
@rust-bors
Auto merge of #142720 - yotamofek:pr/rustdoc/ext-crate-cleanup, r=<try>
4328207 
De-dup common code from `ExternalCrate` methods

<!-- homu-ignore:start -->
<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r? <reviewer name>
-->
<!-- homu-ignore:end -->
Also, return an `impl Iterator` instead of collecting into a `Vec`. Not sure if that'll have a measurable perf impact, but I think this PR still cleans up the two methods it touches quite nicely.

(I'm having trouble finding a name for the common method I extracted, currently called `foobar`, would love suggestions!)
@rust-bors
Copy link
Contributor

rust-bors bot commented Jun 19, 2025

⌛ Trying commit aca0688 with merge 4328207…

To cancel the try build, run the command @bors2 try cancel.

@rust-bors
Copy link
Contributor

rust-bors bot commented Jun 19, 2025

☀️ Try build successful (CI)
Build commit: 4328207 (43282071d47981a2bb3732840c437b432b410909, parent: 2fcf1776b9ccef89993dfe40e9f5c4908e2d2d48)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

rust-timer commented Jun 20, 2025

Finished benchmarking commit (4328207): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -2.0%, secondary -4.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -2.0% [-2.0%, -2.0%] 1
Improvements ✅
(secondary)		 -4.6% [-4.6%, -4.6%] 1
All ❌✅ (primary) -2.0% [-2.0%, -2.0%] 1

Cycles

Results (secondary 5.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 5.1% [5.1%, 5.1%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 692.244s -> 691.619s (-0.09%)
Artifact size: 371.99 MiB -> 371.98 MiB (-0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 20, 2025
@yotamofek
Copy link
Contributor Author

yotamofek commented Jun 20, 2025

Secondary improvement is spurious (it's for the opt profile), so perf is neutral, unsurprisingly.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 20, 2025

So all good, thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 20, 2025

📌 Commit aca0688 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@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 Jun 20, 2025
Kobzol added a commit to Kobzol/rust that referenced this pull request Jun 20, 2025
@Kobzol
Rollup merge of rust-lang#142720 - yotamofek:pr/rustdoc/ext-crate-cle…
99b850e 
…anup, r=GuillaumeGomez

De-dup common code from `ExternalCrate` methods

Also, return an `impl Iterator` instead of collecting into a `Vec`. Not sure if that'll have a measurable perf impact, but I think this PR still cleans up the two methods it touches quite nicely.

(I'm having trouble finding a name for the common method I extracted, currently called `foobar`, would love suggestions!)
@Kobzol Kobzol mentioned this pull request Jun 20, 2025
Closed
bors added a commit that referenced this pull request Jun 20, 2025
@bors
Auto merge of #142781 - Kobzol:rollup-l7ng2lf, r=Kobzol
f0dec36 
Rollup of 11 pull requests

Successful merges:

 - #142478 (install docs for each target in different directory)
 - #142629 (Add config builder for bootstrap tests)
 - #142715 (correct template for `#[align]` attribute)
 - #142720 (De-dup common code from `ExternalCrate` methods)
 - #142736 (add issue template for rustdoc)
 - #142743 (rustc-dev-guide subtree update)
 - #142744 (Add a mailmap entry for y21)
 - #142758 (Make sure to rebuild rustdoc if `src/rustdoc-json-types` is changed)
 - #142764 (Convert `ilog(10)` to `ilog10()`)
 - #142767 (Some symbol and PathRoot cleanups)
 - #142769 (remove equivalent new method on context)

r? `@ghost`
`@rustbot` modify labels: rollup
@Kobzol Kobzol mentioned this pull request Jun 20, 2025
Merged
bors added a commit that referenced this pull request Jun 20, 2025
@bors
Auto merge of #142795 - Kobzol:rollup-vvnnjno, r=Kobzol
5526a2f 
Rollup of 10 pull requests

Successful merges:

 - #142629 (Add config builder for bootstrap tests)
 - #142715 (correct template for `#[align]` attribute)
 - #142720 (De-dup common code from `ExternalCrate` methods)
 - #142736 (add issue template for rustdoc)
 - #142743 (rustc-dev-guide subtree update)
 - #142744 (Add a mailmap entry for y21)
 - #142758 (Make sure to rebuild rustdoc if `src/rustdoc-json-types` is changed)
 - #142764 (Convert `ilog(10)` to `ilog10()`)
 - #142767 (Some symbol and PathRoot cleanups)
 - #142769 (remove equivalent new method on context)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6c58f69 into rust-lang:master Jun 20, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 20, 2025
rust-timer added a commit that referenced this pull request Jun 20, 2025
@rust-timer
Unrolled build for #142720
1eaa0b0 
Rollup merge of #142720 - yotamofek:pr/rustdoc/ext-crate-cleanup, r=GuillaumeGomez

De-dup common code from `ExternalCrate` methods

Also, return an `impl Iterator` instead of collecting into a `Vec`. Not sure if that'll have a measurable perf impact, but I think this PR still cleans up the two methods it touches quite nicely.

(I'm having trouble finding a name for the common method I extracted, currently called `foobar`, would love suggestions!)
@yotamofek yotamofek deleted the pr/rustdoc/ext-crate-cleanup branch June 21, 2025 21:15
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jun 23, 2025
@bors
Auto merge of #142795 - Kobzol:rollup-vvnnjno, r=Kobzol
ec31a95 
Rollup of 10 pull requests

Successful merges:

 - rust-lang/rust#142629 (Add config builder for bootstrap tests)
 - rust-lang/rust#142715 (correct template for `#[align]` attribute)
 - rust-lang/rust#142720 (De-dup common code from `ExternalCrate` methods)
 - rust-lang/rust#142736 (add issue template for rustdoc)
 - rust-lang/rust#142743 (rustc-dev-guide subtree update)
 - rust-lang/rust#142744 (Add a mailmap entry for y21)
 - rust-lang/rust#142758 (Make sure to rebuild rustdoc if `src/rustdoc-json-types` is changed)
 - rust-lang/rust#142764 (Convert `ilog(10)` to `ilog10()`)
 - rust-lang/rust#142767 (Some symbol and PathRoot cleanups)
 - rust-lang/rust#142769 (remove equivalent new method on context)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@notriddle notriddle

Labels

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.89.0

Development

Successfully merging this pull request may close these issues.

7 participants

@yotamofek @rustbot @GuillaumeGomez @rust-timer @rust-log-analyzer @bors @notriddle