-
Notifications
You must be signed in to change notification settings - Fork 75
Set RollForward on FSharpLint.Console to latestMajor #704
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.
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
base: master
Are you sure you want to change the base?
Conversation
Hey @Numpsy thanks for the PR!
Can you please include all this great info in the commit message please? commit messages can have body, not just a title. Also, can you bring this to your PR: #687 (comment) This way we know for sure if the bug is fixed or not. |
I tried pulling in those other commits and the build failed with the error from #687 , but in the logs I saw Which looks like it was downloading the artifact for the test 0.24.3 build, but then installing the 0.24.2 build from NuGet. I've tried to fix that by adding |
Failure now looks like the |
Good progress Richard! But why would it throw this error here and not throw it in our SelfCheck CI step? |
A bit of testing locally suggests that there is some difference in behavior depending on if the FSharplint.Core project has been built before linting than if it hasn't (and the additional test step here isn't doing a build, just a restore as it stands). Possibly also related to the observation at #637 (comment) about different results when linting a single .fs file vs. doing the solution - If i run the 0.24.2 release against single files then I get the same warnings as the last CI build here got, whereas linting the built FSharplint.Core project has no warnings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rebase and update to .NET 9?
- name: Setup Paket | ||
run: | | ||
dotnet tool install --global paket --version 7.2.1 | ||
paket restore | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we can remove this now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it
I'll have a look (at this point, setting rollForward to latestMajor might make it roll onto .NET 10, so I'll see what happens when the .NET 10 preview build is installed) |
Hmm, if I try to run the unit tests on the lastest source with the preview .NET 10 SDK installed then I get a few test failures with messages like this:
It seems ok if I set global.json to use the ,NET 9 SDK Edit: Ah, setting DOTNET_ROLL_FORWARD_TO_PRERELEASE fixes those, I'd forgotten about that one |
Now we can test with .NET 10, similar to #748 |
@Numpsy can you rename title of this PR to convey more clearly what it achieves? Seems the current title explains the what but not the why (it explains the solution, but not the problem). |
refs fsprojects#687, whereby it appears to not be able to process .NET 7 or 8 projects when running on .NET 6, and always using the latest version seems to help. It looks like the current roll foward behaviour was added for fsprojects#519 to fix issues where only a newer version than the tool was built with is available, but this change would make it just use the latest all the time (so for example, if you have 6 and 8 installed, then it will use 8 all the time).
refs #687 , whereby it appears to not be able to process .NET 7 or 8 projects when running on .NET 6, and always using the latest version seems to help.
It looks like the current roll foward behaviour was added for #519 to fix issues where only a newer version than the tool was built with is available, but this change would make it just use the latest all the time (so for example, if you have 6 and 8 installed, then it will use 8 all the time).
I'm not sure if there is any potential to break anything that currently works though.