-
Notifications
You must be signed in to change notification settings - Fork 156
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
Simplify the implementations of hasOrd
and hasEq
#3735
Conversation
@TimSheard another nice trick! |
@lehins there is a bug in fourmolu - it's suggested change does not have the same meaning as the original code and fourmolu knows this. However, the CI still fails. |
@MaximilianAlgehed Can you report it as a bug? It can either be a bug in ormolu or fourmolu, so you'd need to try applying a snippet using latest versions of both of those tools and report it on the appropriate repo, I suspect it will be Should be as simple as downloading the ormolu binary from the repo release and applying it to the snippet that has the semantic broken by it. |
This is the specific error that I got on commit
|
I checked the issue tracker over at the fourmolu repo and a very similar issue has been marked as resolved fourmolu/fourmolu#340. Before I go digging around with this - are we sure we are using the latest version of fourmolu in this repo or are we on a version from before this fix? |
Good point. We are not. I'll submit a PR in a moment that updates it. So, you might be able to remove this commit d2daee7 |
8017d04
to
fd52383
Compare
@MaximilianAlgehed Just in case you didn't notice #3742 fixed the issue with outdated version of fourmolu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is way too cool! I love it ❤️
fd52383
to
f4efa53
Compare
@lehins that updated version of fourmolu doesn't appear to have fixed the issue. Perhaps the fix hasn't been released yet? I can put back the work-around for now with a note to follow this up in the future. |
Before putting workarounds, I'd suggest verifying the problem is not fix and submitting an issue with a reproducer case on fourmolu or ourmolu repo, depending who's responsible for the problem |
f4efa53
to
42dd7cf
Compare
55c06bb
to
c7afed5
Compare
@MaximilianAlgehed could you rebase this on master? We've had a PR merged. FYI, now that you have write permissions to the repo, if you create branches on this repo, instead of a fork, I'd be able to do rebases for you right in the github UI, which would remove this inconvenient friction. |
c7afed5
to
56becaa
Compare
Yes, I'll build my future PRs on this repos origin |
Description
This simplifies the implementation by relying on a generalization of the
IsTypeable
dictChecklist
.cabal
andCHANGELOG.md
files according to theversioning process.
.cabal
files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)CHANGELOG.md
for the affected packages. New section is never added with the code changes. (See RELEASING.md)fourmolu
(usescripts/fourmolize.sh
)scripts/cabal-format.sh
)hie.yaml
has been updated (usescripts/gen-hie.sh
)