Add net481 support - #4227
Conversation
| | DotNetFramework FrameworkVersion.V4_7_1 -> [ DotNetFramework FrameworkVersion.V4_7; DotNetStandard DotNetStandardVersion.V2_0 ] | ||
| | DotNetFramework FrameworkVersion.V4_7_2 -> [ DotNetFramework FrameworkVersion.V4_7_1 ] | ||
| | DotNetFramework FrameworkVersion.V4_8 -> [ DotNetFramework FrameworkVersion.V4_7_2 ] | ||
| | DotNetFramework FrameworkVersion.V4_8_1 -> [ DotNetFramework FrameworkVersion.V4_7_2 ] |
There was a problem hiding this comment.
I think there is a missing fallback to 4_8 here.
There was a problem hiding this comment.
rather, 4_8 should replace 4_7_2 here
There was a problem hiding this comment.
Changed as requested. Tough I do not fully understand, what that functions intent is…
There was a problem hiding this comment.
I'm not 100% clear, but it is setup like a fallback, and has this comment:
// returns a list of compatible platforms that this platform also supports
I always followed the same pattern of putting the previous version, without skipping or adding more than necessary, I assume some code does a directed graph, and that it allows .net framework v4.8 to be ok with packages that only say v3.5 for example.
Without the adjustment you just made, it may not be possible to reference v4.8 in v4.8.1.
Disclaimer: I've not followed how this is used in the codebase.
|
@ChrSteinert could you add one test similar to Otherwise, if CI gives similar output as #4219, I think we can move this forward. |
|
also this one: tests/Paket.Tests/Versioning/PlatformMatchingSpecs.fs |
Add tests for the dependencies-file parser.
|
Thanks for your guidance and review!
…________________________________
Von: Gauthier Segay ***@***.***>
Gesendet: Thursday, September 28, 2023 7:43:04 AM
An: fsprojects/Paket ***@***.***>
Cc: Christian Steinert ***@***.***>; Mention ***@***.***>
Betreff: Re: [fsprojects/Paket] Add net481 support (PR #4227)
@forki<https://github.com/forki>, I checked the flakytests CI outputs, they look similar to #4219<#4219> AFAICS (beside it pulls dotnet 7 instead of 6).
The added tests cover the support for 4.8.1, I think it is good to go in prerelease mode.
—
Reply to this email directly, view it on GitHub<#4227 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACLJHRKANBXHXLBQETVCCYDX4UE6RANCNFSM6AAAAAA5FZWKA4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
/run merge.and.ship.it |
3 participants
This aims to add support for net481 framework restriction, and thus resolve #4224 .
The required change seems to be more intricate, than what has changed so far. For instance, there is something about the NuGetPackageCache.CurrentCacheVersion, that I am not sure about, what to do. It also touched a lot of moving parts (and tests) that I adjusted straight forward, and might be missing some things.
Therefore any advice is apreciated.