Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Remove upper bound on deepseq #49

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

snoyberg commented Nov 29, 2011

The upper bound on deepseq is a bit of an annoyance, as it forces us into cabal dependency hell all the time. This patch uses containers-deepseq to keep the NFData instance of Map even with deepseq 1.2. The instance definitions are taken directly from deepseq 1.1, so there should be no change in behavior.

Collaborator

hvr commented Nov 29, 2011

Have you noticed b84e358 btw?

Contributor

snoyberg commented Nov 29, 2011

No I hadn't, thanks for pointing it out. But if I'm not mistaken, that change will only land in the 0.4 release. Is there a chance of getting that back-ported to the 0.3 series?

Owner

bos commented Nov 30, 2011

I don't really like the containers-deepseq idea, especially with an aeson 0.4 release around the corner. So as much as I appreciate the patch, I must decline. Thanks.

@bos bos closed this Nov 30, 2011

Contributor

snoyberg commented Nov 30, 2011

Even with aeson 0.4, odds are we're going to need to support 0.3 for a bit. If I provided a patch that solves the issue without an additional dependency, would that be up for consideration?

Owner

bos commented Nov 30, 2011

Yep, something like what aeson 0.4 does would be cool.

Just beware that I think that the 0.3.2.12 tag might be associated with the wrong revision. I don't know which the correct one is :-(

On Nov 29, 2011, at 20:52, Michael Snoymanreply@reply.github.com wrote:

Even with aeson 0.4, odds are we're going to need to support 0.3 for a bit. If I provided a patch that solves the issue without an additional dependency, would that be up for consideration?


Reply to this email directly or view it on GitHub:
#49 (comment)

Contributor

snoyberg commented Nov 30, 2011

I just applied my patches against the 0.3.2.12 available on Hackage, here's the diff: https://gist.github.com/1408290

I tried to follow the commit that @hvr mentioned above, but the Tip and Bin constructors do not appear to be exported. Instead, I just used toList like deepseq 1.1 does.

Owner

bos commented Nov 30, 2011

OK, I did a binary search and found the revision that corresponds most closely to 0.3.2.12, patched your change on top as 2eced49, and issued 0.3.2.13. Please let me know if it does the trick for you.

Contributor

snoyberg commented Nov 30, 2011

That's perfect, thank you! I just did a full install of Yesod from scratch, with a deepseq >= 1.2 constraint, and didn't run into any dependency hell.

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