Use version comparison function #41102
Conversation
ghost
added
|
I looked at the first two failures. The first was where it wasn't throwing for 8.0.203.1, which seems like a valid version (now) to me, so I'm planning to just delete that test. The second was this: So when it tried to sort 8.0.203.1, 8.0.203, and 8.0.200-rc.1, it decided 8.0.203 was the max? I don't see how that makes sense... Will look into it |
src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
Show resolved
Hide resolved
| firstDash = firstDash < 0 ? first.Length : firstDash; | ||
| secondDash = secondDash < 0 ? second.Length : secondDash; | ||
|
|
||
| var firstVersion = new Version(first.Substring(0, firstDash)); |
There was a problem hiding this comment.
what happens if you pass in the string without removing the - part first? The dash is supported in versions but I recall there being a reason you needed to do it this way (comment)?
There was a problem hiding this comment.
Well, I tried a fairly standard version with a - in a console project I use for testing, and it threw an exception. The dash is supported in ReleaseVersions, I think, but not Version? Or perhaps there are some strings with dashes that can be parsed into a Version while others can't?
| var modifiedFirst = "1.1.1" + (firstDash == first.Length ? string.Empty : first.Substring(firstDash)); | ||
| var modifiedSecond = "1.1.1" + (secondDash == second.Length ? string.Empty : second.Substring(secondDash)); | ||
|
|
||
| return new ReleaseVersion(modifiedFirst).CompareTo(new ReleaseVersion(modifiedSecond)); |
There was a problem hiding this comment.
Does release version have any limitations? Curious why you can't just compare the part after the dash directly rather than making it into a release version first?
There was a problem hiding this comment.
Well, it needed some digits before the dash to make a ReleaseVersion, but it couldn't support four '.'-separated numbers. I originally proposed doing a simpler comparison like a string comparison, since my current understanding is that the part after the dash is just compared alphanumerically, but dsplaisted was concerned that there might be more complexity and suggested doing it with a ReleaseVersion.
src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
Show resolved
Hide resolved
Creates a version comparison function that permits comparing four-part versions or versions with semantic parts on the end. Uses that function to find the 'max' version of all available workload sets.
I'm planning to try to write a test tomorrow.