Fix error linting projects that use .NET Framework

fsprojects / FSharpLint

Lint tool for F#

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

MIT License

303 stars 73 forks source link

Fix error linting projects that use .NET Framework #657

Closed webwarrior-ws closed 9 months ago

webwarrior-ws commented 9 months ago

Upgrade Ionide.ProjInfo to 0.58.0 (needed to bring the fix for parsing legacy projects, upstream PR: https://github.com/ionide/proj-info/pull/131 , upstream commit: https://github.com/ionide/proj-info/commit/3542cee96562b1e64c20a6b88278b1a8346d83f3 ).
Upgrade FSharp.Compiler.Service to version 41 (needed because the new Ionide.ProjInfo version, depends on a higher version of F.C.S.).
Made changes to framework, rules and test framework code necessary to accomodate API changes in this version 41.x of FCS compared to version 40.

Fixes https://github.com/fsprojects/FSharpLint/issues/336

knocte commented 9 months ago

Upgrade FSharp.Compiler.Service to version 41.

Please explain why this is needed.

Upgrade Ionide.ProjInfo to 0.58.0.

Please explain why this is needed.

knocte commented 9 months ago

Upgrade FCS to version 41 and Ionide.ProjInfo to 0.58.0

Title of PR should reflect the goal of the PR, not its implementation details.

webwarrior-ws commented 9 months ago

Upgrade FSharp.Compiler.Service to version 41.

Please explain why this is needed.

Upgrade Ionide.ProjInfo to 0.58.0.

Please explain why this is needed.

Ionide.ProjInfo upgrade is needed to fix https://github.com/fsprojects/FSharpLint/issues/336. FSharp.Compiler.Service upgrade is needed because of Ionide.ProjInfo upgrade.

knocte commented 9 months ago

Ionide.ProjInfo upgrade is needed...

I'm not asking you to explain it to me, because I already know (and I know you know). I'm asking you to edit the PR description.

knocte commented 9 months ago

Fix error linting projects that use newer .NET versions

Newer .NET versions??? Please read the github issue first.

webwarrior-ws commented 9 months ago

Weren't there several issues that required new Ionide.ProjInfo? I remember testing on .NET 6 project.

knocte commented 9 months ago

Weren't there several issues that required new Ionide.ProjInfo? I remember testing on .NET 6 project.

We didn't confirm that new versions of Ionide.ProjInfo fixed other problems. But you did confirm that it fixed issue #336, so that reason alone (only it) is enough to merge this PR.