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

replace !Vec::is_empty() then Some(Vec) with bool::then() #14516

Closed
wants to merge 1 commit into from

Conversation

srtnnm
Copy link

@srtnnm srtnnm commented Sep 8, 2024

What does this PR try to resolve?

None of existing issues.

Just replace if !vec.is_empty() {Some(vec)} with (!vec.is_empty()).then(|| vec) in src/bin/cargo/commands/rustc.rs
looks a little better i think

@rustbot
Copy link
Collaborator

rustbot commented Sep 8, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2024
@srtnnm srtnnm changed the title replace !Vec::is_empty() then Some with bool::then() replace !Vec::is_empty() then Some(Vec) with bool::then() Sep 8, 2024
@@ -80,23 +80,15 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
compile_opts.build_config.requested_profile = InternedString::new("dev");
}
let target_args = values(args, "args");
compile_opts.target_rustc_args = if target_args.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

It has to be wrapped with additional parentheses so doesn't seem more readable to me. The clippy lint if_then_some_else_none also only hints for the pattern of if-some-else-none not for the reversed.

Is there any other purpose of this change? Otherwise I tend to close this. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I've found that if ! is used, I would rather have an if / else if

@bors
Copy link
Contributor

bors commented Sep 9, 2024

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

@weihanglo
Copy link
Member

Thank you. I am going to close this. See #14516 (comment).

@weihanglo weihanglo closed this Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. Command-rustc 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