Extract ClassyPrelude #8
Conversation
library/Magicbane.hs
Outdated
| -- from ClassyPrelude | ||
|
|
||
| -- | Originally 'Conc.yield'. | ||
| yieldThread :: MonadBase IO m => m () |
library/Magicbane.hs
Outdated
| @@ -38,6 +36,15 @@ import Magicbane.Metrics as X | |||
| import Magicbane.Validation as X | |||
| import Magicbane.HTTPClient as X | |||
| import Magicbane.Util as X | |||
| -- replacing ClassyPrelude | |||
There was a problem hiding this comment.
- please don't add the "replacing ClassyPrelude" comment
- no need to reexport everything that ClassyPrelude offered! only things used in Magicbane itself!!
- move these imports to the top, they shouldn't be under
Magicbane.*imports
(same in other files)
There was a problem hiding this comment.
Comment was just FYI to highlight changes, I expected it to be going away before merge. :)
The only thing I explicitly held on to is the Concurrent/Async stuff, because per your comments on master, there are combinations that don't work with ExceptT. Seemed worth asserting one that does. I guess this could just be clarified in docs, tho... then yieldThread goes away too (Classy renames that to avoid namespace conflict with conduit/pipes, I was just preserving the whole conceptual group as-is).
There was a problem hiding this comment.
diff itself is a highlight of changes :)
Oh okay I guess that makes sense, maybe keep yieldThread but document why it's there
| @@ -1,5 +1,5 @@ | |||
| {-# OPTIONS_GHC -fno-warn-missing-signatures #-} | |||
| {-# LANGUAGE NoImplicitPrelude, NoMonomorphismRestriction, OverloadedStrings, UnicodeSyntax, DataKinds, TypeOperators, MultiParamTypeClasses, TypeFamilies, FlexibleContexts, FlexibleInstances, UndecidableInstances, GeneralizedNewtypeDeriving, CPP #-} | |||
There was a problem hiding this comment.
Don't remove OverloadedStrings anywhere. Even if there are no strings in the file, I think it should be enabled everywhere.
There was a problem hiding this comment.
This should all be addressed now. I split out the concurrency stuff to Magicbane.Async for clarity of intent, and updated the readme to list lifted-async among the included packages.
|
A couple of new changes, both in package.yaml: I bumped the version to 0.2.0 (because breaking change), and commented out a few dependencies that are unused, to reduce the transient dependency footprint. (Initially removed them entirely, but they look like things you may have plans for.) |
I don't have any plans, I'm not actively working on Haskell stuff right now :) |
| @@ -42,19 +47,19 @@ dependencies: | |||
| - wai | |||
| - wai-middleware-metrics | |||
| - wai-cli | |||
| - servant | |||
| # - servant | |||
There was a problem hiding this comment.
woah wait what?! servant is like the main thing here
There was a problem hiding this comment.
It surprised me too, but servant-server is where the Servant module is actually defined, and it re-exports everything Magicbane directly uses. Granted it depends on servant anyway so this isn't saving any build time.
There was a problem hiding this comment.
oh, right — I didn't even notice the servant-server on the next line. facepalm.
|
Are you planning on any more changes or should I merge it? |
|
Unless you want me to fully delete those commented packages, I'm done! Verified that it works out of the box with RIO now, too. |
|
Yeah, cool. Thanks! |
|
Not to be a pain, but a Hackage release would be handy! |
|
Sure. Released 0.2.0. |
Addressing #7