Conversation
|
👋 @9999years Reviewer: Please verify the following things have been done, if applicable.
|
|
Looks like we'll need to drop GHC 8.10 support in order to support 9.4.2 - should be fine to delete that part of CI and raise our own lower bound on |
| import Data.List (intercalate) | ||
| import Distribution.Package (packageVersion) | ||
| import Distribution.PackageDescription.Parsec (readGenericPackageDescription) | ||
| import Distribution.Simple.PackageDescription (readGenericPackageDescription) |
There was a problem hiding this comment.
ahhh this will probably fail when built with earlier Cabal - maybe,
#if MIN_VERSION_Cabal(3,8,0)
import Distribution.Simple.PackageDescription (readGenericPackageDescription)
#else
import Distribution.PackageDescription.Parsec (readGenericPackageDescription)
#endif
(not sure if this is built by all GHC or just most recent)
|
Thanks for this! We usually wait for Ormolu to merge changes, and periodically merge Ormolu's master branch in to Fourmolu. Often this is when they make a release. But even if we stick to that process, I'm sure your work here is useful and can be reused. |
|
Thanks! Like @georgefst said, we usually wait for Ormolu to merge. While git should hopefully merge it correctly, if it doesnt, the conflicts can get ugly. Is there any particular reason to merge this into Fourmolu immediately, or is it ok to wait? Another reason is that it'd be nice to wait until we can actually test with 9.4 in CI. |
|
We use upstream PRs at mercury to track changes in our internal code - we're upgrading to GHC 9.4.2 now, and we need fourmolu to work on it. |
There were a fair amount of conflicts getting this PR ported to fourmolu's codebase, but it didn't take more than a few hours to resolve them. |
|
@parsonsmatt Does that require a merge, it would it simply need this PR to remain open? @9999years sure, but then the conflicts merging ormolu's changes into fourmolu's changes that already partially incorporates ormolu's changes would be annoying |
Based on @amesgen's branch here: https://github.com/tweag/ormolu/tree/amesgen/ghc-9.4 tweag/ormolu@0a3da23
|
The upstream PR has been merged but not released yet. Do we want to wait for a release to be cut before landing this? |
|
We're also integrating these changes into |
|
For the reasons I mentioned in my last comment, merging this will cause annoying conflicts when merging Ormolu. Hopefully Ormolu makes a release soon and we'll be able to merge it in soon after. If this happens soon enough, we can just close this PR, as it'll be incorporated in the PR merging Ormolu |
Based on @amesgen's branch here:
tweag/ormolu#885
https://github.com/tweag/ormolu/tree/amesgen/ghc-9.4
tweag/ormolu@0a3da23