-
Notifications
You must be signed in to change notification settings - Fork 56
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
Support GHC 9.10 #262
base: master
Are you sure you want to change the base?
Support GHC 9.10 #262
Conversation
This turned out to be more complicated than anticipated, this change only fixes the top-level error, now the build presently fails with:
|
On second thought, most of the errors weren't that bad, we are down to the following:
|
This is not the prettiest thing, but at least this is now compiling with ghc-9.10.1! Marking this as ready for review. |
016dafe
to
b98d8e6
Compare
Arg, unfortunately the generated code doesn't compile, this PR produces: @(HsProtobuf.UnpackedVec HsProtobuf.String Hs.Text) … instead of: @(HsProtobuf.UnpackedVec (HsProtobuf.String Hs.Text)) Would someone know how that works? Since that's the only failure we got, it can be fixed with: |
Probably we need to explicitly add parentheses. That isn't automatic. There are also some changes to the Nix code that will be required; I'm investigating those. |
@TristanCacqueray , I hope you don't mind but for reasons of speed I filed a separate PR to try to make the required Nix changes: #263 . If it works out and is approved then it should subsume this one, and I presume that as long as GHC 9.10 is correctly supported that satisfies your requirements. In any case #263 should provide some additional results from continuous integration. |
@TristanCacqueray , curiously, the Nix environment I set up in #263 doesn't end up using aeson-2.2, so I didn't end up needing to incorporate those changes just to get it to build. |
Ah, because we did some extra work to stay on the old version. I'll think about this... |
Right, this aeson came with the latest nixpkgs, for the record, here is the change where we bumped to 9.10: change-metrics/monocle#1142 |
No description provided.