Skip to content

Improve wording for suggestion messages #12170

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

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

GuillaumeGomez
Copy link
Member

Follow-up of #12140.

r? @llogiq

changelog: Improve wording for suggestion messages

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 18, 2024
@GuillaumeGomez GuillaumeGomez force-pushed the improve-suggestions-wording branch 2 times, most recently from bc64bee to 998ba82 Compare January 18, 2024 19:39
@GuillaumeGomez
Copy link
Member Author

Fixed CI.

@smoelius
Copy link
Contributor

smoelius commented Jan 18, 2024

Please forgive me for chiming in. I was planning to submit a PR to remove all the uses of "consider".

Here is a concrete example:

"consider implementing `IntoIterator` instead",

IMHO, "consider" doesn't add anything here, and just the following would suffice:

"implement `IntoIterator` instead", 

EDIT: Do you not agree? Do you think "consider" adds something?

@GuillaumeGomez
Copy link
Member Author

It's possible that the clippy suggestion is either wrong or not the best in a given context. Using "consider" leaves this possibility open and tells the users "it might be better than what you already have, so maybe consider using it?". At least that's how I see it.

@smoelius
Copy link
Contributor

It's possible that the clippy suggestion is either wrong or not the best in a given context. Using "consider" leaves this possibility open ...

OK. (Sorry for jumping on your PR.)

@GuillaumeGomez
Copy link
Member Author

It's fine don't worry. Better discuss before changes are merged to ensure everyone is on board. :)

@llogiq
Copy link
Contributor

llogiq commented Jan 20, 2024

Looks good to me. Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 20, 2024

📌 Commit 998ba82 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 20, 2024

⌛ Testing commit 998ba82 with merge bf2a738...

bors added a commit that referenced this pull request Jan 20, 2024
…llogiq

Improve wording for suggestion messages

Follow-up of #12140.

r? `@llogiq`

changelog: Improve wording for suggestion messages
@bors
Copy link
Contributor

bors commented Jan 20, 2024

💔 Test failed - checks-action_test

@GuillaumeGomez GuillaumeGomez force-pushed the improve-suggestions-wording branch from 998ba82 to 38d9585 Compare January 20, 2024 15:49
@GuillaumeGomez
Copy link
Member Author

I ran the tests locally and it all worked. Not sure what happened... I rebased just in case.

@y21
Copy link
Member

y21 commented Jan 20, 2024

something's up with the macos runners, they have been randomly failing in a bunch of PRs for a few days now
https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Sporadic.20macos.20failures

so it probably just needs a retry

@llogiq
Copy link
Contributor

llogiq commented Jan 20, 2024

@bors retry

@y21
Copy link
Member

y21 commented Jan 20, 2024

Hmm seems like that wasn't picked up by bors? Maybe it needs to be reapproved since a new commit was added/history was changed because of the rebase? 🤔

@llogiq
Copy link
Contributor

llogiq commented Jan 21, 2024

Let's try that then.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 21, 2024

📌 Commit 38d9585 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 21, 2024

⌛ Testing commit 38d9585 with merge 99423e8...

@bors
Copy link
Contributor

bors commented Jan 21, 2024

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

@bors bors merged commit 99423e8 into rust-lang:master Jan 21, 2024
@GuillaumeGomez GuillaumeGomez deleted the improve-suggestions-wording branch January 21, 2024 13:44
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.

6 participants