Skip to content

Derive Copy for MatchOptions struct #71

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 15 commits into from
Feb 5, 2019
Merged

Derive Copy for MatchOptions struct #71

merged 15 commits into from
Feb 5, 2019

Conversation

g-s-k
Copy link
Contributor

@g-s-k g-s-k commented Jan 18, 2019

MatchOptions contains 3 boolean values, which are Copy. It would be nice to be able to pass it by-value (rather than borrow it or create a new one on each function call).

Also included in this PR:

  • Removed a compile warning due to a deprecated trait (::std::ascii::AsciiExt)
  • Turned on clippy::pedantic and fixed all of its recommendations (don't worry, turned it off afterward)
  • Turned off a test for macos that counts on a /root directory (my install does not have one)

@KodrAus
Copy link
Contributor

KodrAus commented Jan 21, 2019

Hi @g-s-k 👋

Thanks for taking the time to do some cleanup here. I'll just have to check that we haven't bumped the minimum version of rustc that's needed to build the library too far because of any clippy suggestions.

@g-s-k
Copy link
Contributor Author

g-s-k commented Jan 21, 2019

@KodrAus good point - please let me know if/how i can help with that.

@g-s-k
Copy link
Contributor Author

g-s-k commented Feb 1, 2019

hi @KodrAus - just wanted to bump this. I installed rust stable 1.24.0 (the current release the last time a PR was merged) and cargo test ran without any errors. Is there an earlier version you'd like me to test?

@g-s-k
Copy link
Contributor Author

g-s-k commented Feb 2, 2019

I've done a more thorough check now. Here's what I found:

  • 1.23.0 and later compile and test successfully
  • 1.17.0 through 1.22.0 require the AsciiExt trait to be in scope
  • 1.14.0 through 1.16.0 require variable names that match struct field names to be repeated after the colon (foo: foo,)
  • 1.13.0 requires a lifetime for const string slices
  • 1.12.0 and earlier don't recognize the format of the Cargo.lock.

All but the last line indeed apply to this PR - how far back do you need to remain compatible?

@KodrAus
Copy link
Contributor

KodrAus commented Feb 5, 2019

@g-s-k Sorry for dropping the ball on this one, and thanks for checking this out!

1.23.0 is over 12 months old, so I think that's a reasonable minimum version to start supporting.

Copy link
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks again for cleaning this up!

@KodrAus KodrAus merged commit 1b5b670 into rust-lang:master Feb 5, 2019
@g-s-k
Copy link
Contributor Author

g-s-k commented Feb 5, 2019

Happy to help!

@g-s-k g-s-k deleted the clone-derive branch February 5, 2019 01:29
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.

2 participants