Compatibility with hpqtypes 1.13.0.0 #46
Conversation
arybczak
force-pushed
the
hpqtypes-1.13.0.0
branch
from
bcb857c to
db61ca6
Compare
Raveline
left a comment
Raveline
left a comment
There was a problem hiding this comment.
Looks generally good to me. I got a bit sidetracked when reading because your withState is not at all behaving like the withState from mtl and I guess it's a bit confusing for people who are used to the mtl one, but since it's a purely internal function, it doesn't matter.
| { unDBEff :: Eff (State (DBState es) : es) a | ||
| } | ||
| deriving newtype (Functor, Applicative, Monad, MonadThrow, MonadCatch, MonadMask) | ||
| runDB cs0 ts0 m = PQ.withConnectionData cs0 ts0 $ \cd0 -> do |
There was a problem hiding this comment.
Lots of do in this block that are not necessary.
There was a problem hiding this comment.
Yeah, as I said before, this is for appeasing emacs indentation mode :p
| runDB | ||
| :: forall es a | ||
| . IOE :> es | ||
| :: IOE :> es |
There was a problem hiding this comment.
How come you can now remove the forall here now ? Or was it already unnecessary to begin with ?
There was a problem hiding this comment.
Old runWithState and doWithTransaction from the where clause was using it before.
| GetLastQuery :: DB m SomeSQL | ||
| GetTransactionSettings :: DB m TransactionSettings | ||
| SetTransactionSettings :: TransactionSettings -> DB m () | ||
| GetLastQuery :: DB m (BackendPid, SomeSQL) |
There was a problem hiding this comment.
As you very often don't care about the first parameter (as illustrated in the change you made in tests), it might be interesting to add a helper getLastQuery' which basically fmap snd over getLastQuery. It's likely to be a very common use-case.
There was a problem hiding this comment.
Not quite. getLastQuery is never used in regular code, it's pretty much a helper for throwDB to fill in details in the DBEXception wrapper.
arybczak
force-pushed
the
hpqtypes-1.13.0.0
branch
from
a214a6a to
26eac22
Compare
Needs scrive/hpqtypes#80.