Skip to content

Cleaning lints#4525

Merged
bors merged 1 commit intorust-lang:masterfrom
lukaslueg:springclean
Sep 23, 2017
Merged

Cleaning lints#4525
bors merged 1 commit intorust-lang:masterfrom
lukaslueg:springclean

Conversation

@lukaslueg
Copy link
Contributor

I've started to clean some minor defects in cargo. This is the first commit of possibly many.

Requesting advice if this is actually wanted; #cargo was positive.

Some things raise the minimum version of rust required to compile cargo. E.g. assert_ne!(foo, bar) instead of assert!(foo != bar) requires (iirc) rust 1.13. Any advice on that in particular?

@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@matklad
Copy link
Contributor

matklad commented Sep 22, 2017

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Sep 22, 2017

📌 Commit 33bcccb has been approved by alexcrichton

@lukaslueg
Copy link
Contributor Author

@alexcrichton, I understand that implicitly raising the minimal rust version required to compile cargo is a non-problem?

@alexcrichton
Copy link
Member

Indeed!

@bors
Copy link
Contributor

bors commented Sep 22, 2017

⌛ Testing commit 33bcccb with merge f5fafbe241a77c26cc04b14c655d6faac5ebbced...

let cfg_color = self.get_string("term.color").unwrap_or(None).map(|v| v.val);

let color = color.as_ref().or(cfg_color.as_ref());
let color = color.as_ref().or_else(|| cfg_color.as_ref());
Copy link
Contributor

Choose a reason for hiding this comment

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

there are a bunch of .foo(|| someFunc()) in this PR - can any of them be rewritten by passing the function directly: .foo(someFunc)? In this case, that would be color.as_ref().or_else(cfg_color.as_ref).

Note that in this specific case something even better is possible: color.or(cfg_color).as_ref()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, that was an oversight. I have many more changes waiting and will incorporate this in the next PR.

@bors
Copy link
Contributor

bors commented Sep 23, 2017

💥 Test timed out

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Sep 23, 2017

⌛ Testing commit 33bcccb with merge 168a4ea...

bors added a commit that referenced this pull request Sep 23, 2017
Cleaning lints

I've started to clean some minor defects in cargo. This is the first commit of possibly many.

Requesting advice if this is actually wanted; #cargo was positive.

Some things raise the minimum version of rust required to compile cargo. E.g. `assert_ne!(foo, bar)` instead of `assert!(foo != bar)` requires (iirc) rust 1.13. Any advice on that in particular?
@bors
Copy link
Contributor

bors commented Sep 23, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 168a4ea to master...

@bors bors merged commit 33bcccb into rust-lang:master Sep 23, 2017
@bors bors mentioned this pull request Sep 23, 2017
@ehuss ehuss added this to the 1.22.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants