Skip to content
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

[#39] Build with GHC 8.2.2 and GHC 8.6.1 on CI #52

Merged
merged 4 commits into from Oct 4, 2018

Conversation

brandonhamilton
Copy link
Contributor

Closes #39

Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @brandonhamilton! Thanks a lot for your contribution!

CI is still on but I can tell that as we are supporting GHC starting from 8.2.2 and up to 8.4.3 now we should change the base lower bound accordingly – base >= 4.10 && < 4.13. So this version will not break in the future.

@vrom911 vrom911 added Hacktoberfest https://hacktoberfest.digitalocean.com/ CI labels Oct 4, 2018
@vrom911 vrom911 added this to In progress in #2: Hacktoberfest (October, 2018) via automation Oct 4, 2018
@chshersh chshersh changed the title Build with GHC 8.2.2 and GHC 8.6.1 on CI [#39] Build with GHC 8.2.2 and GHC 8.6.1 on CI Oct 4, 2018
@@ -28,7 +28,7 @@ library
Colog.Core.Class
Colog.Core.Severity

build-depends: base >= 4.11 && < 5
build-depends: base >= 4.10 && < 5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point to make it < 4.13 is that when the new base version will be released we still can guarantee that this version of our library will work as expected. If we specify < 5 this would mean that the breaking changes of base-4.13.* or later can break this particular version of our library and we will need to make a revision on Hackage to tighten the base bounds to < 4.13. I hope this explanation makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that makes sense 👍

#2: Hacktoberfest (October, 2018) automation moved this from In progress to Reviewer approved Oct 4, 2018
Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thank you very much, this is very useful to have!

@brandonhamilton
Copy link
Contributor Author

Aha - there is now a build issue on 8.2.2

@brandonhamilton
Copy link
Contributor Author

Yes thanks; you've done all the work in figuring out the cause 😉
I will push a fix to this PR

@chshersh
Copy link
Contributor

chshersh commented Oct 4, 2018

@brandonhamilton Sorry, turned out I was wrong (so I've deleted my comment). The old interface was only in the GHC 8.0.2 and the new one appeared in GHC 8.2.2. So this is not actually a reason of error...

@chshersh
Copy link
Contributor

chshersh commented Oct 4, 2018

@brandonhamilton Okay, looks like GHC 8.2.2 has total disrespect to forall in MessageField constructor. Writing instance like this solves the issue:

    fromLabel field = TM.WrapTypeable $ MessageField @_ @fieldName field

So I think you still can use -XCPP and resolve this particular issue. But then you will observe the compilation error with README.lhs...

Copy link
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All builds are passing now! 🎉

@chshersh chshersh merged commit 7e0f738 into co-log:master Oct 4, 2018
#2: Hacktoberfest (October, 2018) automation moved this from Reviewer approved to Done: PR's Oct 4, 2018
@chshersh chshersh removed this from Done: PR's in #2: Hacktoberfest (October, 2018) Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Hacktoberfest https://hacktoberfest.digitalocean.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants