search

IntersectMBO / bech32

Haskell implementation of the Bech32 address format (BIP 0173).
Apache License 2.0
47 stars 13 forks source link

Use cabal-3.4.0.0 #32

Closed newhoggy closed 3 years ago

newhoggy commented 3 years ago

Also:

newhoggy commented 3 years ago

Could you explain why 3.4.0.0 is better for caching?

If we used git references in our cabal.project, then caching is better. Although we don't do this in the project, so it probably doesn't help. I've removed "better caching" from the title of the PR.

KtorZ commented 3 years ago

It is not okay to disable tests. If tests don't pass on 8.10.3 then we do not support 8.10.3.

So either we fix the test suite / the code where appropriate or we restrict ghc's version.

I'll open a ticket to track this chunk or work and prioritize it. @newhoggy may you provide some context (here or on Slack) about this and why it is necessary?

newhoggy commented 3 years ago

To clarify, the tests already fail on ghc-8.10.2.

We currently do not build or test ghc-8.10.2 in CI in this project, but all downstream projects do.

This PR is mainly to confirm that ghc-8.10.3 builds.

I suspect fixing tests for ghc-8.10.2 will also allow tests under ghc-8.10.3 to pass.

I hope to eventually see if ghc-8.10.3 fixes some Windows build issues we've had previously, but projects need to build first because I can make that confirmation: https://gitlab.haskell.org/ghc/ghc/-/issues/18946

KtorZ commented 3 years ago

@newhoggy I know that tests fail on 8.10.2 and that's also why we do not build for ghc-8.10.2 either. Same remarks apply here :+1:

hasufell commented 3 years ago

Maybe we misunderstood @newhoggy and this PR isn't meant to be merged as of now, but just to trigger the builds?

newhoggy commented 3 years ago

The PR is mergible.

I think that having ghc-8.10.x built in CI with no tests is superior to ghc-8.10.x not being built at all because the point of CI is to prevent regressions and catching build errors is still useful. This PR includes some changes that demonstrates a way to do that.

However, if this is a blocker, I can remove the ghc-8.10.x part of the PR.

KtorZ commented 3 years ago

There's no point using a library if the tests aren't actually passing. Period.

So the PR cannot be merged. We'll eventually allocate some time to look into the test suite and fix whatever needs fixing.

See: https://jira.iohk.io/browse/ADP-640

newhoggy commented 3 years ago

Thanks for the input. I've removed the problem changes so the PR no longer builds for ghc-8.10.2.

newhoggy commented 3 years ago

Thanks!