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

Optionally allow expect and unwrap in tests #8802

Merged
merged 2 commits into from
May 8, 2022

Conversation

smoelius
Copy link
Contributor

@smoelius smoelius commented May 8, 2022

This addresses #1015, except it makes the new behavior optional.

The reason for the msrv-related changes is as follows.

Rather than expand check_methods list of arguments, it seemed easier to make check_methods a method of Methods, so that check_methods could access Methods' fields.

check_methods had an msrv parameter, which I consequently made a field of Methods. But, to avoid adding a lifetime parameter to Methods, I made the field type Option<RustcVersion> instead of the parameter's existing type, Option<&RustcVersion>. This seemed sensible since RustcVersion implements Copy. But this broke a lot of code that expected an Option<&RustcVersion> or &Option<RustcVersion>. I changed all of those occurrences to Option<RustcVersion>. IMHO, the code is better as a result of these changes, though.

The msrv-related changes are in their own commit to (hopefully) ease review.

Closes #1015

changelog: optionally allow expect and unwrap in tests

r? @llogiq

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 8, 2022
@llogiq
Copy link
Contributor

llogiq commented May 8, 2022

Yeah, the & and .as_ref()s are noisy. I doubt it'll make much of a difference perf-wise. Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented May 8, 2022

📌 Commit 597f61b has been approved by llogiq

@bors
Copy link
Collaborator

bors commented May 8, 2022

⌛ Testing commit 597f61b with merge 4667198...

@bors
Copy link
Collaborator

bors commented May 8, 2022

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

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.

Disable result_unwrap_used / option_unwrap_used in tests
4 participants