-
Notifications
You must be signed in to change notification settings - Fork 124
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
Update regex #638
Update regex #638
Conversation
@Dylan-DPC I think there might be some misunderstanding of how semver works.
this basically means, than if i push your change and regex go up to 1.6 I will not have this change. Since regex is at version >=1 that would include fixes but no breaking changes. Also cargo install ALWAYS takes the highest value possible based on dependencies. so if you do cargo-install today (without your changes) 1.5.5 would be used anyway. so unless i'm missing something, this change doesn't add anything and will not impact new users doing cargo install but will limit them in the future once regex goes above 1.5 feel free to reopen if I missed anything and this is needed. for now, based on my understanding, it is not. |
no, this is false. My change does
Yes you are right but this isn't always guaranteed, there could be a chance (in future say) that you include a dependency (even if transitive) then with the current setup you might end up with a version that you don't intend to. This just pushes the baseline and ensures that doesn't happen |
got it. good point. |
@@ -59,7 +59,7 @@ home = "^0.5" | |||
indexmap = { version = "^1", features = ["serde-1"] } | |||
ignore = "^0.4" | |||
log = "^0.4" | |||
regex = "^1" | |||
regex = "1.5.5" |
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.
i know its default in cargo, but its not the semver way. i'm not sure why cargo decided that by default ^ is implied even if not written, but i feels its not readable.
can we just add the ^ so it will be ^1.5.5
@Dylan-DPC can we just add the: ^ there? i know cargo does it for us, but its not the semver way and i feel its confusing for non rust people. |
Codecov Report
@@ Coverage Diff @@
## master #638 +/- ##
=======================================
Coverage 93.67% 93.68%
=======================================
Files 112 112
Lines 21112 21127 +15
=======================================
+ Hits 19777 19792 +15
Misses 1335 1335
Continue to review full report at Codecov.
|
@Dylan-DPC I'll update the version there. in mean time, i'm merging this. thanks |
Update regex to
1.5.5
as per security advisory