-
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
Rewrite testEql using Typeable to make it impossible to forget cases #3697
Conversation
@lehins and @TimSheard it would be good to get this reviewed and merged. |
17cd3ed
to
48181e4
Compare
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 awesome. I love how IsTypeable
solves this source of mistakes when adding new Rep constructors: https://github.com/input-output-hk/cardano-ledger/blob/9e2f8151e3b9a0dde9faeb29a7dd2456e854427c/libs/cardano-ledger-test/src/Test/Cardano/Ledger/Constrained/TypeRep.hs#L417
libs/cardano-ledger-test/src/Test/Cardano/Ledger/Constrained/TypeRep.hs
Outdated
Show resolved
Hide resolved
48181e4
to
1e4f19b
Compare
@lehins could you please run the checks? I think this is the final version of the PR. |
@MaximilianAlgehed This PR needs to be rebases on master. Whenever you see this at the bottom of the PR this is necessary for merging:
|
1e4f19b
to
11a2120
Compare
@lehins done |
Head branch was pushed to by a user without write access
11a2120
to
425e003
Compare
@lehins something appears to be broken in CI. |
@MaximilianAlgehed Looks like some issue with Github since yesterday. Traballs don't get downloaded fully or corrupted. This happens on multiple repos that we have.
|
425e003
to
325eae3
Compare
@MaximilianAlgehed Looks like Github has finally back to normal operation since yesterday. Just in case I implemented a fallback workaround too in #3713 Do you mind rebasing again on master again? We've had a few PRs merged yesterday. |
325eae3
to
c58c75f
Compare
@lehins still broken? |
@MaximilianAlgehed Why do you say it is broken? CI hasn't finished yet. (Only "Required" ones are important) |
Right... So the one thats called |
It is relevant, but it just doesn't work for forked repos, I think. In any case it is not terribly reliable, but it is useful, since it runs CI in nix, where GithubActions is build with regular cabal setup without nix. |
Perfect, thanks for clearing that up. I won't care about that one in the
future when working from our fork.
Den tors 7 sep. 2023 12:41Alexey Kuleshevich ***@***.***>
skrev:
… Right... So the one thats called ci is irrelevant to CI?
It is relevant, but it just doesn't work for forked repos, I think. In any
case it is not terribly reliable, but it is useful, since it runs CI in
nix, where GithubActions is build with regular cabal setup without nix.
—
Reply to this email directly, view it on GitHub
<#3697 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBPPQN4JAPPOSFDWLVURDDXZGQETANCNFSM6AAAAAA4HJFO4I>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Description
Everything is
Typeable
so we can use that to reduce some clutter.Checklist
.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
)