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

aeson-pretty: use Spaces with confIndent #1431

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@ilovezfs

ilovezfs commented Jul 3, 2016

Based on https://github.com/informatikr/aeson-pretty/blob/375afe8e/Data/Aeson/Encode/Pretty.hs#L124

Prevents the following error building Elm 0.17.1 with GHC 8.0.1:

    • No instance for (Num Json.Indent) arising from the literal ‘2’
    • In the ‘confIndent’ field of a record
      In the expression:
        Json.Config
          {Json.confIndent = 2, Json.confCompare = Json.keyOrder keys}
      In an equation for ‘config’:
          config
            = Json.Config
                {Json.confIndent = 2, Json.confCompare = Json.keyOrder keys}
            where
                keys = ["tag", ....]
aeson-pretty: use Spaces with confIndent
Based on https://github.com/informatikr/aeson-pretty/blob/375afe8e/Data/Aeson/Encode/Pretty.hs#L124

Prevents the following error building Elm 0.17.1 with GHC 8.0.1:
    • No instance for (Num Json.Indent) arising from the literal ‘2’
    • In the ‘confIndent’ field of a record
      In the expression:
        Json.Config
          {Json.confIndent = 2, Json.confCompare = Json.keyOrder keys}
      In an equation for ‘config’:
          config
            = Json.Config
                {Json.confIndent = 2, Json.confCompare = Json.keyOrder keys}
            where
                keys = ["tag", ....]
@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Jul 3, 2016

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Jul 3, 2016

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jul 15, 2016

Contributor

That error would also be prevented by constraining aeson-pretty to a version < 0.8 in the .cabal file.

Contributor

jvoigtlaender commented Jul 15, 2016

That error would also be prevented by constraining aeson-pretty to a version < 0.8 in the .cabal file.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Jul 15, 2016

And does this PR break it with that older version?

ilovezfs commented Jul 15, 2016

And does this PR break it with that older version?

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jul 15, 2016

Contributor

Yes.

Contributor

jvoigtlaender commented Jul 15, 2016

Yes.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Jul 15, 2016

Then that dep should be bumped.

ilovezfs commented Jul 15, 2016

Then that dep should be bumped.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jul 15, 2016

Contributor

That's for @evancz to decide.

Contributor

jvoigtlaender commented Jul 15, 2016

That's for @evancz to decide.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Jul 15, 2016

I said it should be bumped, not that it will be ;)

ilovezfs commented Jul 15, 2016

I said it should be bumped, not that it will be ;)

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jul 15, 2016

Member

Since no one has confirmed that this code works with GHC 7.10.2 which I am using, I can't accept this PR for now. Not planning to do that kind of upgraded right now.

I have 0.7.2 of aeson-pretty, so if putting an upper bound on that package fixes it for GHC 8, I'd much rather go with that approach.

Member

evancz commented Jul 15, 2016

Since no one has confirmed that this code works with GHC 7.10.2 which I am using, I can't accept this PR for now. Not planning to do that kind of upgraded right now.

I have 0.7.2 of aeson-pretty, so if putting an upper bound on that package fixes it for GHC 8, I'd much rather go with that approach.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jul 15, 2016

Member

Gonna go with the constraint instead. Thanks for bring the issue up and suggesting a fix!

Member

evancz commented Jul 15, 2016

Gonna go with the constraint instead. Thanks for bring the issue up and suggesting a fix!

@evancz evancz closed this Jul 15, 2016

bradediger added a commit to bradediger/nixpkgs that referenced this pull request Aug 4, 2016

garbas added a commit to NixOS/nixpkgs that referenced this pull request Aug 4, 2016

@gentoid

This comment has been minimized.

Show comment
Hide comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment