Skip to content
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

Fix clippy lints in librustdoc #89430

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

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
src/librustdoc/clean/auto_trait.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/auto_trait.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
src/librustdoc/core.rs Show resolved Hide resolved
src/librustdoc/formats/cache.rs Show resolved Hide resolved
notriddle
notriddle requested changes Oct 1, 2021
View reviewed changes
src/librustdoc/clean/auto_trait.rs Outdated Show resolved Hide resolved
src/librustdoc/passes/collect_intra_doc_links.rs Outdated
@@ -1497,7 +1497,7 @@ fn range_between_backticks(ori_link: &MarkdownLink) -> Range<usize> {
.bytes()
.skip(after_first_backtick_group)
.position(|b| b == b'`')
.unwrap_or(ori_link.link.len());
.unwrap_or_else(|| ori_link.link.len());
Copy link
Contributor

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

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

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

Choose a reason for hiding this comment

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

It was reverted already. ;)

src/librustdoc/passes/html_tags.rs Outdated Show resolved Hide resolved
src/librustdoc/html/sources.rs Outdated Show resolved Hide resolved
@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
@GuillaumeGomez
Copy link
Member Author

Updated!

@bors
Copy link
Contributor

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

Rebased.

@GuillaumeGomez
Copy link
Member Author

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

@bors: r=jyn514,camelid,notriddle

@bors
Copy link
Contributor

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
Contributor

bors commented Oct 25, 2021

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

@camelid
Copy link
Member

camelid commented Oct 25, 2021

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

@bors
Copy link
Contributor

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

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

@jyn514 jyn514 jyn514 requested changes

@notriddle notriddle notriddle approved these changes

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