Set RollForward on FSharpLint.Console to latestMajor

fsprojects / FSharpLint

Lint tool for F#

https://fsprojects.github.io/FSharpLint/

MIT License

300 stars 73 forks source link

Set RollForward on FSharpLint.Console to latestMajor #704

Open Numpsy opened 3 months ago

Numpsy commented 3 months ago

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.

knocte commented 3 months ago

Hey @Numpsy thanks for the PR!

refs https://github.com/fsprojects/FSharpLint/issues/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 https://github.com/fsprojects/FSharpLint/issues/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.

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: https://github.com/fsprojects/FSharpLint/issues/687#issuecomment-1949979932 This way we know for sure if the bug is fixed or not.

Numpsy commented 3 months ago

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 --prerelease to the install command in the test so that it installs the test build

Numpsy commented 3 months ago

Failure now looks like the testReleaseBinariesInWindows8 step needs to do a paket restore before running the tool?

Numpsy commented 3 months ago

Ok, so it now seems to be running the tool ok, but failing the build due to an error exit code, which is something i suppose:

knocte commented 3 months ago

Good progress Richard! But why would it throw this error here and not throw it in our SelfCheck CI step?

Numpsy commented 3 months ago

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 https://github.com/fsprojects/FSharpLint/pull/637#discussion_r1493377735 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.