-
Notifications
You must be signed in to change notification settings - Fork 975
feat: use semver to match required version
#6066
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
feat: use semver to match required version
#6066
Conversation
ytmimi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this one so soon. I'll be a little busy this coming weekend so I wanted to at least get my initial review done now. Good stuff so far!
…n' of github.com:ologbonowiwi/rustfmt into feat/rust-lang#6063/use-semver-to-check-required-version
|
changes addressed @ytmimi. Thanks for the quick feedback 😄 |
ytmimi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more notes, but we're moving in the right direction
Co-authored-by: Yacin Tmimi <[email protected]>
…n' of github.com:ologbonowiwi/rustfmt into feat/rust-lang#6063/use-semver-to-check-required-version
…lang#6063/use-semver-to-check-required-version
Co-authored-by: Yacin Tmimi <[email protected]>
…n' of github.com:ologbonowiwi/rustfmt into feat/rust-lang#6063/use-semver-to-check-required-version
ea85318 to
4e15dbf
Compare
ytmimi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor note about rewording the #### Multiple version to match docs and maybe adding another test case for the wildcard match.
|
appreciate the feedback and patience @ytmimi, great rewording on those docs. lmk if there's anything else 🎉 |
ytmimi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just re-reviewed the changes. Thanks again for sticking with this 🎉
required_versionsemversemverto compare versionssemverrequired_versionparsing manuallyCloses #6063