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

Remove direct dependency on deprecated library #242

Conversation

newhoggy
Copy link

@newhoggy newhoggy commented Jul 22, 2021

This PR is related to pcapriotti/optparse-applicative#428 making it possible for optparse-applicative evolve in a way that incurs less breakage, but I think PR itself also has it own merits.

@RyanGlScott
Copy link
Member

Thanks for the PR, and for confirming that this is in relation to ongoing work in optparse-applicative, which is helpful context to keep in mind.

One thing that I feel is worth pointing out is that, strictly speaking, this isn't removing the ansi-wl-pprint dependency like the PR title suggests. Rather, it's removing a direct import of ansi-wl-pprint's API in favor of an indirect import from optparse-applicative, which re-exports it. Generally speaking, I'm wary of using re-exported APIs, since if the underlying API undergoes breaking changes and the library which re-exports it does not change its version number, then it's impossible to guard against the problem by using cabal version bounds. (This is a problem which has arisen before.)

Why am I mentioning this? It's because pcapriotti/optparse-applicative#428 aims to replace the very API that this part of optparse-applicative is re-exporting. It's possible that when the dust settles on that PR, the API will be 100%, bit-for-bit compatible with the old one. But then again, it's also possible that this won't be the case—backwards compatibility is a tricky thing. As a result, I'm currently inclined to wait to see the final shape that pcapriotti/optparse-applicative#428 takes on before moving on this. Does that sound agreeable?

@newhoggy newhoggy changed the title Remove dependency on deprecated library Remove direct dependency on deprecated library Jul 22, 2021
@newhoggy
Copy link
Author

newhoggy commented Jul 22, 2021

Yes, that makes sense. I have fixed up the title of the PR.

You are right to be wary of re-exports.

I hope for pcapriotti/optparse-applicative#428 to preserve compatibility for the usage here, hence the exploration I've taken in this PR.

Hopefully when the dust settles this PR will be able to merge without risk of breaking changes.

@RyanGlScott
Copy link
Member

Superseded by #275 and #276.

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

Successfully merging this pull request may close these issues.

2 participants