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.

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

New lint format_add_strings #8626

Merged
merged 1 commit into from
Apr 14, 2022
Merged

New lint format_add_strings #8626

merged 1 commit into from
Apr 14, 2022

Conversation

pitaj
Copy link
Contributor

@pitaj pitaj commented Apr 4, 2022

Closes #6261

changelog: Added [format_add_string]: recommend using write! instead of appending the result of format!

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 4, 2022
@pitaj pitaj force-pushed the format_add_string branch 2 times, most recently from 290b7d3 to 4d5103f Compare April 4, 2022 04:03
@bors
Copy link
Collaborator

bors commented Apr 4, 2022

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

Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

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

This looks good to me. I have but one small suggestion and you will need to rebase the changes. Once that's done, I'll be happy to merge.

clippy_lints/src/format_push_string.rs Outdated Show resolved Hide resolved
@pitaj
Copy link
Contributor Author

pitaj commented Apr 5, 2022

Changed to recommend as _ for import and rebased.

@bors
Copy link
Collaborator

bors commented Apr 11, 2022

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

@llogiq
Copy link
Contributor

llogiq commented Apr 14, 2022

Thanks! It will need another rebase, then it'll be ready to merge. Feel free to r+ it yourself

@bors delegate+

@bors
Copy link
Collaborator

bors commented Apr 14, 2022

✌️ @pitaj can now approve this pull request

@pitaj
Copy link
Contributor Author

pitaj commented Apr 14, 2022

@llogiq why does it need another rebase? There are no conflicts.

@xFrednet
Copy link
Member

xFrednet commented Apr 14, 2022

Maybe he missed your push? I'd say we let bors decide if this is ready or not. I don't want to steal your honer to use bors. You can merge this by commenting @bors r=llogiq

It looks like bors will merged this regardless of the inline code block...

@bors

This comment was marked as outdated.

@bors

This comment was marked as outdated.

bors added a commit that referenced this pull request Apr 14, 2022
New lint `format_add_strings`

Closes #6261

changelog: Added [`format_add_string`]: recommend using `write!` instead of appending the result of  `format!`
@xFrednet

This comment was marked as outdated.

@bors
Copy link
Collaborator

bors commented Apr 14, 2022

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: 576f561 (576f561d5280fad64dd22b38f8cadba042a66e11)

@pitaj
Copy link
Contributor Author

pitaj commented Apr 14, 2022

@bors r=llogiq

@bors
Copy link
Collaborator

bors commented Apr 14, 2022

📌 Commit 67badbe has been approved by llogiq

@bors
Copy link
Collaborator

bors commented Apr 14, 2022

⌛ Testing commit 67badbe with merge aade96f...

@bors
Copy link
Collaborator

bors commented Apr 14, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing aade96f to master...

@bors bors merged commit aade96f into rust-lang:master Apr 14, 2022
@bors bors mentioned this pull request Apr 14, 2022
@pitaj pitaj deleted the format_add_string branch April 14, 2022 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn on push_str(&format!())
5 participants