Skip to content

Fix clippy lints in librustdoc #89430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 25, 2021
Merged

Fix clippy lints in librustdoc #89430

merged 1 commit into from
Oct 25, 2021

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