Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

servant-foreign: Derive Data for all types #809

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
4 participants
@ocharles
Copy link
Contributor

ocharles commented Sep 20, 2017

I would like to be able to find all "foreign" types under the list of requests generated by listFromApi. With a Data instance, it's trivial to repeatedly walk the [Req MyLangType], and use cast to see if I'm looking at a MyLangType or not.

I also moved some StandaloneDeriving clauses to just a normal deriving clause. If there was a reason you want these to remain standalone I can change it back.

@alpmestan

This comment has been minimized.

Copy link
Contributor

alpmestan commented Sep 20, 2017

That should be absolutely fine, LGTM. @phadej no objection I suppose?

@@ -49,7 +49,6 @@ library
, GeneralizedNewtypeDeriving
, MultiParamTypeClasses
, ScopedTypeVariables
, StandaloneDeriving
, TemplateHaskell

This comment has been minimized.

Copy link
@phadej

phadej Sep 20, 2017

Member

Add DeriveDataTypeable here?

@ocharles

This comment has been minimized.

Copy link
Contributor Author

ocharles commented Sep 20, 2017

Done.

@@ -9,6 +9,7 @@ module Servant.Foreign.Internal where

import Control.Lens (makePrisms, makeLenses, Getter, (&), (<>~), (%~),
(.~))
import Data.Data

This comment has been minimized.

Copy link
@phadej

phadej Sep 20, 2017

Member

seems the unrestricted import breaks -Werror as Data.Data exports Proxy too. Could you use explicit import lists, maybe then travis will be happy?

@jkarni

This comment has been minimized.

Copy link
Member

jkarni commented Oct 3, 2017

I'm not sure I understand the 7.8.4 CI failure, but a guess would be adding Typeable as well?

@phadej phadej added this to the 0.12 milestone Nov 6, 2017

@phadej phadej self-assigned this Nov 6, 2017

@phadej

This comment has been minimized.

Copy link
Member

phadej commented Nov 6, 2017

I'll take ownership on this. Will correct & merge soon.

@phadej

This comment has been minimized.

Copy link
Member

phadej commented Nov 6, 2017

superseded by #846

@phadej phadej closed this Nov 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.