Add more instances. #7
Conversation
|
Notes:
|
|
(I don’t squash and merge; fine-grained history is fine by me.) |
| version: 0.0.0.0 | ||
| version: 0.1.0.0 |
There was a problem hiding this comment.
I don’t think this constitutes a major release; there’s no backwards-compat issue that I can see.
There was a problem hiding this comment.
New instances being exported may conflict with orphan instances defined in user code.
There was a problem hiding this comment.
You could make the same claim for any added symbol—it could break a build if you’re importing it open in a module using a symbol of the same name from elsewhere.
There was a problem hiding this comment.
Other sorts of names are safe to add assuming the consumers follow the advised practice of importing qualified or with explicit import lists. On the contrary, the import of classes is not possible to opt out.
There was a problem hiding this comment.
Actually, the official guide for PVP says that added non-orphan instances are not considered a breaking change. These instances here are non-orphan. So you are right.
| @@ -50,6 +50,8 @@ library | |||
| build-depends: | |||
| , base >= 4.10 && < 5 | |||
| , transformers ^>= 0.5.6.2 | |||
| , generic-data ^>= 0.9 | |||
There was a problem hiding this comment.
I’d rather we define the instance ourselves than take on this dependency.
| EQ -> liftCompare f v v' | ||
| x -> x | ||
|
|
||
| instance NFData (f a) => NFData (Interval f a) |
There was a problem hiding this comment.
NFData1 exists too, FWIW. Let’s define both.
| instance Show1 f => Show1 (Interval f) where | ||
| liftShowsPrec = gliftShowsPrec |
There was a problem hiding this comment.
In addition to not wanting to take on the extra dependency, it strikes me as strange that we’d have different behaviour for Show1 and Show.
| import Control.Monad.Trans.Class | ||
| import Data.Coerce (coerce) | ||
| import Data.Fixed (mod') | ||
| import Data.Function (on) | ||
| import Data.Functor.Classes (Show1, liftShowsPrec, Eq1, liftEq, Ord1, liftCompare) |
There was a problem hiding this comment.
I don’t think we need to list the imports explicitly here, but if we’re going to, let’s sort this alphabetically. I think I have stylish-haskell config in this repo, if that helps.
|
|
||
| ## `0.1.0.0` — Sunday, 13 of September 2020. | ||
|
|
||
| * Add instances `Show1`, `Eq1`, `Ord1`. |
|
Thank you for your thoughtful commentary. I shall get it fixed. |
Resolve #5.