Skip to content

Fix clippy lints in librustdoc#89430

Merged
bors merged 1 commit intorust-lang:masterfrom
GuillaumeGomez:rustdoc-clippy-lints
Oct 25, 2021
Merged

Fix clippy lints in librustdoc#89430
bors merged 1 commit intorust-lang:masterfrom
GuillaumeGomez:rustdoc-clippy-lints

Conversation

@GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Oct 1, 2021

I ran clippy on librustdoc and simply fixed the lints. :)

r? @notriddle

@GuillaumeGomez GuillaumeGomez 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 Oct 1, 2021
@GuillaumeGomez GuillaumeGomez reopened this Oct 1, 2021
jyn514
jyn514 suggested changes Oct 1, 2021
View reviewed changes
notriddle
notriddle requested changes Oct 1, 2021
View reviewed changes
src/librustdoc/passes/collect_intra_doc_links.rs Outdated
Copy link
Contributor

@notriddle notriddle Oct 1, 2021

Choose a reason for hiding this comment

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

Calling len() on a string is cheap. This seems like a poor suggestion.

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Oct 1, 2021

Choose a reason for hiding this comment

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

Micro optimization one would say: String::len calls self.vec.len() (which simply returns its len field). Well, lint fixed at least haha.

Copy link
Member

@jyn514 jyn514 Oct 13, 2021

Choose a reason for hiding this comment

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

@GuillaumeGomez can you please revert this? I don't think it's an improvement.

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Oct 14, 2021

Choose a reason for hiding this comment

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

It was reverted already. ;)

@GuillaumeGomez GuillaumeGomez added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 1, 2021
@bors

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc-clippy-lints branch 2 times, most recently from f7b1ef0 to b0d8544 Compare October 5, 2021 09:56
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Oct 5, 2021 •
edited
Loading

Updated! I also reverted some changes too as discussed previously.

@bors

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
camelid
camelid reviewed Oct 9, 2021
View reviewed changes
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Oct 11, 2021

Updated!

@bors
Copy link
Collaborator

bors commented Oct 23, 2021

☔ The latest upstream changes (presumably #90203) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Oct 25, 2021

Rebased.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Oct 25, 2021

Since I reverted everything that needed to be, let's move forward!

@bors: r=jyn514,camelid,notriddle

@bors
Copy link
Collaborator

bors commented Oct 25, 2021

📌 Commit 4614ca4 has been approved by jyn514,camelid,notriddle

@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 Oct 25, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 25, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 25, 2021
@matthiaskrgr
Rollup merge of rust-lang#89430 - GuillaumeGomez:rustdoc-clippy-lints…
bd61c4d 
…, r=jyn514,camelid,notriddle

Fix clippy lints in librustdoc

I ran clippy on librustdoc and simply fixed the lints. :)

r? `@notriddle`
@bors
Copy link
Collaborator

bors commented Oct 25, 2021

⌛ Testing commit 4614ca4 with merge 29b1248...

camelid
camelid reviewed Oct 25, 2021
View reviewed changes
@camelid
Copy link
Member

camelid commented Oct 25, 2021

@bors rollup=iffy (some of these changes may have perf implications)

@bors
Copy link
Collaborator

bors commented Oct 25, 2021

☀️ Test successful - checks-actions
Approved by: jyn514,camelid,notriddle
Pushing 29b1248 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 25, 2021
@bors bors merged commit 29b1248 into rust-lang:master Oct 25, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 25, 2021
This was referenced Oct 25, 2021
Closed
Merged
Merged
Merged
@rust-timer
Copy link
Collaborator

rust-timer commented Oct 26, 2021

Finished benchmarking commit (29b1248): 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

@camelid
Copy link
Member

camelid commented Oct 26, 2021 •
edited
Loading

Consistent improvements of around 0.05% on instruction count for doc builds. max-rss is mixed, but very possibly spurious.

@GuillaumeGomez GuillaumeGomez mentioned this pull request Oct 26, 2021
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 26, 2021
@matthiaskrgr
Rollup merge of rust-lang#90302 - GuillaumeGomez:rm-unneeded-into_ite…
5feb198 
…r, r=jyn514

Remove unneeded into_iter

As `@camelid` mentionned [here](rust-lang#89430 (comment)), the `into_iter` was unneeded.

r? `@camelid`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 26, 2021
@matthiaskrgr
Rollup merge of rust-lang#90302 - GuillaumeGomez:rm-unneeded-into_ite…
76392fb 
…r, r=jyn514

Remove unneeded into_iter

As ``@camelid`` mentionned [here](rust-lang#89430 (comment)), the `into_iter` was unneeded.

r? ``@camelid``
@GuillaumeGomez GuillaumeGomez deleted the rustdoc-clippy-lints branch August 19, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@camelid camelid camelid left review comments

@notriddle notriddle notriddle approved these changes

+1 more reviewer

@jyn514 jyn514 jyn514 requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. 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.58.0

Development

Successfully merging this pull request may close these issues.

7 participants

@GuillaumeGomez @bors @camelid @rust-timer @notriddle @jyn514 @rustbot