criterion icon indicating copy to clipboard operation
criterion copied to clipboard

Remove direct dependency on deprecated library

Open newhoggy opened this issue 3 years ago • 2 comments

This PR is related to https://github.com/pcapriotti/optparse-applicative/pull/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.

newhoggy avatar Jul 22 '21 01:07 newhoggy

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?

RyanGlScott avatar Jul 22 '21 10:07 RyanGlScott

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 https://github.com/pcapriotti/optparse-applicative/pull/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.

newhoggy avatar Jul 22 '21 10:07 newhoggy

Superseded by #275 and #276.

RyanGlScott avatar May 23 '23 23:05 RyanGlScott