Map isless(a,b) to <(a,b) #139
Conversation
|
I think you should be able to drop all other comparison operators. Can you try that? |
src/WinRPM.jl
Outdated
| @@ -321,11 +321,7 @@ struct RPMVersionNumber | |||
| end | |||
| Base.convert(::Type{RPMVersionNumber}, s::AbstractString) = RPMVersionNumber(s) | |||
| Base.:(<)(a::RPMVersionNumber, b::RPMVersionNumber) = false | |||
| Base.:(==)(a::RPMVersionNumber, b::RPMVersionNumber) = true | |||
There was a problem hiding this comment.
I don't think you can remove this definition. Have you tested it? OTC, is < really needed?
(I must say these constant definitions are really weird...)
There was a problem hiding this comment.
I did test it on all operations except for install because it is still failing due to a follow-on error. I can put it back for now until I can test end-to-end without any issues.
< will be needed for backwards compatibility since maximum() calls < and not isless in .6
There was a problem hiding this comment.
I also agree they are really weird. I think because there is a lot of repetition in the package list it is just trying to grab the first or last entry which represents the oldest or newest and really isn't doing any number based comparison.
There was a problem hiding this comment.
this package is undertested, difficult to test, but heavily depended on, so please minimize the changes you make to it to be as low risk as possible. particularly if you are fixing 0.7-only issues, don't mix concerns and risk breaking 0.6.
There was a problem hiding this comment.
I agree with @tkelman I rolled my changes back to just adding isless to get .7 to work.
RandomString123
force-pushed
the
ltfix
branch
from
03411bc
to
51717db
Compare
|
@tkelman Merge if that's OK? |
To fix #138 and ensure no breakage mapping an isless call to less than.