SwensenSoftware / unquote

Write F# unit test assertions as quoted expressions, get step-by-step failure messages for free
http://www.swensensoftware.com/unquote
Apache License 2.0
285 stars 26 forks source link

Reduce FSharp.Core dep version? #169

Open bartelink opened 2 months ago

bartelink commented 2 months ago

See https://github.com/SwensenSoftware/unquote/pull/166#issuecomment-2039170789 TL;DR could the FSharp.Core dep be lowered back down to 6.0.7 (or lower) please?

I'll make a PR soon, unless this is being done intentionally for some reason?


Updating the FSharp.Core causes a lot of run on implications For example: https://github.com/fsprojects/Argu/pull/237 This is not dissimilar to how FsHttp also did a similar update: https://github.com/fsprojects/FsHttp/issues/183 FSharp.AWS.DynamoDB and other low level components depend on unquote TaskSeq follows this approach too: https://github.com/fsprojects/FSharp.Control.TaskSeq/pull/123

My personal stuff depends on 6.0.7 for two reasons

NOTE: I do get that there's no actual reason why some old thing from 10 yars ago can't 'Just' update it's FSharp.Core from 4.3.2 to 8.0.100 without expecting it to just work, but that's not the point - it should simply depend on the lowest version possible

stephen-swensen commented 2 months ago

Hi @bartelink - I haven't looked at this closely yet, but some thoughts:

Just thinking out loud. It might be that at this point in time, FSharp.Core 6.0.7 is indeed a sweet spot. I think I'd need to have a better understanding of what changes there are between FSharp.Core 6.0.7 and 8.0.100, specifically with regard to quotations to make sure we don't limit our feature set coverage.

bartelink commented 2 months ago

Hm, good points. It's admittedly a pretty wierd thing to be concerned about as being on the latest in the context of an app is a no-brainer. I guess it's libs like Argu and FsAwsDdb that wind up with the real choices to make. I'm reluctant to up e.g. Argu's requirements as you don't know what upping the requirement winds up transitively causing a mess - I just know that the FsHttp thing cost us time (though it was the incorrect nuspect that actually caused that pain).

Obviously multitargeting is definitely something to avoid, but as an idea, doing a netstandard build that only argets v4 or v6 and then e.g. a net6.0-specific TFM build that demands 8.0 might be a middle way.

For me, the main issue is that I use unquote for tests, and I have them on latest. Amusingly, latest for me is net6.0 atm. So that probably means I'm some bugfixes behind. For Argu, that means dependabot spam like the one I cited, which is suggesting to up the lib's dependency (which is not something I want), but being up to date for tests is fine. Anyway that's more about me and depenadabot than unquote.

One final thing though:

or you miss out on important bug fixes like Async.Parallel you mentioned, or you become more likely to have your dependency surface a security vulnerability

I think that misses the point though - apps should have the latest of everything; and for libs, forcing that is rarely a consideration (I was just giving an example of an exception to the rule, but then I also hadn't even considered the need to cover new language things as I typed that)

bartelink commented 2 months ago

I did the basic PR in #170 FWIW it also compiles against 4.7.2, as 6.x did (and 4.3.2 as Argu does) I've just noticed that FSharp.AWS.DynamoDB does not pin FSharp.Core - I'm off to make a PR to make that as conservative as possible...