Conversation
| List.revIter (Binding >> add) bindings | ||
| // let! or use! | ||
| | SynExpr.LetOrUse(_, _, _, true, bindings, leftHandSide, _, _) -> | ||
| match bindings with |
There was a problem hiding this comment.
Change based on the FCS release notes at https://github.com/dotnet/fsharp/blob/c1b13e3d4542d1de7439b34de00384934365aeb9/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md?plain=1#L63
e5c21fc to
032f006
Compare
6b9014d to
6ab840a
Compare
|
Not sure why the docs build failed with the build using the RTM FCS 43.10 when it worked with the RC2 version |
|
Or is that just because the CI has started installing .NET 10 on it's own when it didn't this morning?
Apparently the use-dotnet action always installs the latest version now, which is .NET 10 as of earlier today - actions/setup-dotnet#674 |
|
@Numpsy you're right, our daily CI run has started to fail today. Maybe setup-dotnet doesn't honor |
|
@Numpsy actually, I don't think it's a bug in setup-dotnet action because all other CI jobs work except the docs one. So maybe it's a bug in Fornax? Fornax not honoring global.json? |
|
It sounds like it's running with the .NET 9 SDK but trying to build as .NET 10 for some reason? (though the error shows the 9.0.306 SDK, and global.json is 9.0.201). |
There does seem to be some machinery at https://github.com/ionide/Fornax/blob/master/src/Fornax/FSIRefs.fs for fishing for runtime libraries, so maybe you're right there and something is going wrong with .NET 10 |
|
I tried changing the CI to actually run as .NET 10 and that fails with a type check error in the build script - https://github.com/Numpsy/FSharpLint/actions/runs/19300021169/job/55193408198#step:7:12 Nothing can ever be simple :-( |
Hey @Numpsy I probably managed to fix the above problem with this commit from my fork: knocte@3cc0a65 can you include it in this PR and rebase? |
|
I'll have a look later |
dc7abab to
8bf0fa8
Compare
| fieldPats | ||
| |> List.map (fun ((_, fieldIdent), _, _) -> fieldIdent.idRange) | ||
| |> List.map (fun patPairField -> patPairField.Range) | ||
| // @@TODO@@ Do we need to look at the ranges inside FieldName here? |
There was a problem hiding this comment.
Any comments on this one?
There was a problem hiding this comment.
Not sure TBH, but if unit tests pass without looking at the ranges, then I wouldn't oppose merging this PR before we investigate this. That said, if you can come up with a unit test that can exercise this code, then I'd sleep better. BTW, I'm aiming to merge this PR when FCS 43.10.103 gets released (because that version will contain a fix which now is only in their master branch).
There was a problem hiding this comment.
Ok.
FWIW, 43.10.102 seems to work without any further code changes.
…eing too long" This reverts commit de27b1b.

Just having a go at building it with the RC .NET 10 version to see if it all works.