Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Loosen FSharp.Core version requirement #459

Merged
merged 2 commits into from
Jun 22, 2023
Merged

Loosen FSharp.Core version requirement #459

merged 2 commits into from
Jun 22, 2023

Conversation

JohnTheGr8
Copy link
Contributor

quick PR to change expecto's FSharp.Core version requirement from (= 7.0.200) to (>= 7.0.200 && < 8.0.0)

closes #458

@farlee2121 farlee2121 mentioned this pull request Jun 21, 2023
Closed
@JohnTheGr8
Copy link
Contributor Author

Please let me know if I should take care of the failing security check as part of this PR...

@farlee2121
Copy link
Collaborator

I did a bit of investigating, and the insecure package isn't actually a result of this change. It stems from some build-only dependencies, but the insecure transitive dependency can't currently be resolved just by upgrading direct package dependencies.

I think we're good to solve the two issues separately. However, looking at previous version constraints, I think nuget FSharp.Core >= 6 is probably the constraint we're looking for. I'll let you grab that if you still want the merge. I appreciate you taking initiative to contribute!

@farlee2121
Allow any higher version for FSharp.Core
e4c2e78 
Before the net6.0 upgrade we allowed any FSharp.Core >= 4.6.0.
This is more consistent with that package behavior.
FSharp.Core also upgrades major versions fairly often (once a year), but rarely introduces changes that wouldn't be backwards compatible for us.
@farlee2121
Copy link
Collaborator

Just realized I can edit your PR. Hope that's ok with you.

@farlee2121 farlee2121 merged commit 31343ce into haf:main Jun 22, 2023
@JohnTheGr8
Copy link
Contributor Author

It stems from some build-only dependencies, but the insecure transitive dependency can't currently be resolved just by upgrading direct package dependencies.

Yup, I figured as much... You can trick Paket to update just that package by adding it to paket.dependencies, updating it and then resetting paket.dependencies.

I think nuget FSharp.Core >= 6 is probably the constraint we're looking for.

I tried this initially but it makes no difference: when packing, the minimum required version will be set to what Expecto uses, currently version 7.0.200

@farlee2121
Copy link
Collaborator

I thought about editing the lock file or similar, but it seems like an unstable solution.

Hmm. When I inspected the nuget packages, the ~> approach does result in a max allowed version, while the >= approach does not
image
image

This seems more in line with previous versioning constraints

@JohnTheGr8
Copy link
Contributor Author

image

This is what I'm getting...

@farlee2121
Copy link
Collaborator

Weird. Not sure why we'd get different results.

@JohnTheGr8
Copy link
Contributor Author

are you running an outdated version of nuget package explorer by any chance? .NETCoreApp, Version=v6.0 sounds very odd 😂

@farlee2121
Copy link
Collaborator

Nope. Looks like the latest release was 2022-08, and that's what I've got.

@JohnTheGr8
Copy link
Contributor Author

anyways, just meant to point out that in my testing setting it to >= 6 and >= 7 made no difference as it would always result in >= 7.0.200... doesn't really matter

@farlee2121
Copy link
Collaborator

Noted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Loosen FSharp.Core version requirement
2 participants
@JohnTheGr8 @farlee2121