search

NorfairKing / sydtest

A modern testing framework for Haskell with good defaults and advanced testing features.
115 stars 25 forks source link

sydtest: Compatibility with mtl-2.3 #60

Closed locallycompact closed 1 year ago

locallycompact commented 1 year ago

mtl 2.3 compat

NorfairKing commented 1 year ago

@locallycompact I'm not much fussed about whether this is merged, but CI fails for now.

locallycompact commented 1 year ago

@NorfairKing biff

NorfairKing commented 1 year ago

Would this work without the CPP?

larskuhtz commented 1 year ago

You can remove the CPP, if you are fine with a redundant import warning from GHC with mtl <2.3. Or you could just drop support for older version of mtl -- which may break downstream users. You could also hide the respective symbols from the import of Control.Monad.Reader, but that would probably result in a warning about a redundant hide clause for mtl >=2.3.

It's probably just a matter of personal taste. (Personally, I consider CPP a good solution, because it makes it clear what part of the code is legacy and what is the recent version.)

locallycompact commented 1 year ago

I prefer wider bounds in general. Navigating upgrade paths when bounds are super tight can get really chewy and can lead to a lot of unsolvable build plans, but I also like explicit imports so CPP is the only hack that gets all of those properties.

NorfairKing commented 1 year ago

@locallycompact Alright I'm fine with CPP then. I can't merge until CI passes and there seems to be real breakage.

locallycompact commented 1 year ago

@NorfairKing I think this works now.

NorfairKing commented 1 year ago

Merged manually and released.