-
-
Notifications
You must be signed in to change notification settings - Fork 281
Add new public function Pkg.satisfies_compat(::VersionNumber, ::String)
, and add (to docs) some examples of checking [compat]
entries
#4292
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
base: master
Are you sure you want to change the base?
Conversation
cc: @MilesCranmer |
d16545e
to
01c9f51
Compare
satisfies_compat::VersionNumber, ::String)
, and add (to docs) some examples of checking [compat]
entriesPkg.satisfies_compat::VersionNumber, ::String)
, and add (to docs) some examples of checking [compat]
entries
01c9f51
to
cd1735c
Compare
…g)`, and add (to docs) some examples of checking `[compat]` entries Co-authored-by: Miles Cranmer <[email protected]>
407663e
to
c23ea56
Compare
SGTM +1 |
CHANGELOG entry added. |
I've added a few tests. We also have a few doctests in the docstring. |
Pkg.satisfies_compat::VersionNumber, ::String)
, and add (to docs) some examples of checking [compat]
entriesPkg.satisfies_compat(::VersionNumber, ::String)
, and add (to docs) some examples of checking [compat]
entries
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.
SGTM but worth getting @KristofferC review too
Co-authored-by: Ian Butterworth <[email protected]>
julia> Pkg.satisfies_compat(v"0.1.0", "=0.1") | ||
true | ||
|
||
julia> Pkg.satisfies_compat(v"0.1.0", "=0.1.1") |
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 think we can reduce the number of examples a bit, I think people will understand how to use it after one or two examples.
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.
LGTM, with the note that I think we can reduce the number of examples given a bit.
#1263 wants to get the range returned. Perhaps we can also do that (but only document that you can call |
Could that be a follow-on? I can tidy up the examples to get this merged if so. |
But if we go with that there is no need for the |
Can all semver specs be represented by a single range object?
|
A VersionSpec can contain a collection of ranges. |
So how does the user test inclusion of a version in that collection of ranges? You're proposing returning a range not a VersionSpec? |
A VersionSpec is a single object (that can internally have many ranges in it) , that you can test |
Ok, I was understanding that you wanted to return a single range, not the VersionSpec (which would need to become public, as discussed in #3847 (comment) which lead to this alternative PR)
|
Making VersionSpec public seems fine to me. |
Ah sorry, I should have been more accurate and said |
Co-authored-by: Miles Cranmer
Replacement for #3847 (closes #3847).
This is an alternative for #3847. AFAICT, it satisfies the use case of #3847, and has the advantages of not exposing internal types, and not renaming any internal functions.
I'm not at all married to the name
satisfies_compat
- I'm happy to change it.