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

Review API for conditionally defined items #27

Closed
hvr opened this issue Dec 9, 2016 · 4 comments
Closed

Review API for conditionally defined items #27

hvr opened this issue Dec 9, 2016 · 4 comments
Assignees
Milestone

Comments

@hvr
Copy link
Member

hvr commented Dec 9, 2016

I notice/realise we expose some instances that are defined based on base version despite them existing before the given base version. This is obviously a problem as it would violate the package abstraction in a non-benign way.

@hvr hvr added this to the 1.4.3 milestone Dec 9, 2016
@hvr hvr self-assigned this Dec 9, 2016
@RyanGlScott
Copy link
Member

RyanGlScott commented Dec 9, 2016

Which instances are you referring to? Are you talking about, say, the NFData instance for Array, which has a different context depending on the base version? Are you referring to instances like the one for Identity, which are only exposed conditionally based on the base version but have existed prior to that in transformers? Something else?

@hvr
Copy link
Member Author

hvr commented Dec 12, 2016

@RyanGlScott Among others, I noticed a couple of new NFData1 instances being added and guarded behind CPP. I'd just like to get an overview of the damage, and figure out whether to add some big visible documentation signpost in the haddocks, or even constraint the lower bound on base, in order to reduce the exposure to the risk. I was already planning to constraint to base >= 4.5 so we can finally get rid of some of the CPP... too bad new CPP was added in the meantime ;-)

PS: Of course, in the case of deepseq it's not as bad as I made it sound, since if ppl make use of Travis to check with all the base versions they declare support for, the risk is rather low for such mistakes (assuming a conditionally defined instance exists) sneaking onto Hackage.

@RyanGlScott
Copy link
Member

Ah, #21 you mean. Here are all the conditionally defined NFData{1,2} instances that were introduced:

Down looks actually quite doable on all supported versions of base... I might whip up a PR to take care of that one.

@RyanGlScott
Copy link
Member

What is the current status of this?

@mixphix mixphix closed this as completed Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants