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

Introduce SwaggerItems GADT to simplify ParamSchema #24

Merged
merged 1 commit into from Dec 18, 2015
Merged

Conversation

fizruk
Copy link
Member

@fizruk fizruk commented Dec 13, 2015

Removes one type parameter from ParamSchema and allows to use less type annotations.

For instance, prior to this change this code required extra type annotation and gives a not very helpful type error:

>>> encode $ toParamSchema (Proxy :: Proxy Integer)

<interactive>:5:1:
    No instance for (ToJSON i0) arising from a use of ‘encode’
    The type variable ‘i0’ is ambiguous
    Note: there are several potential instances:
      instance ToJSON ApiKeyLocation
        -- Defined at /Users/nickolaykudasov/git/getshoptv/swagger2/src/Data/Swagger/Internal.hs:719:1
      instance ToJSON ApiKeyParams
        -- Defined at /Users/nickolaykudasov/git/getshoptv/swagger2/src/Data/Swagger/Internal.hs:720:1
      instance ToJSON (CollectionFormat t)
        -- Defined at /Users/nickolaykudasov/git/getshoptv/swagger2/src/Data/Swagger/Internal.hs:840:10
      ...plus 36 others
    In the expression: encode
    In the expression: encode $ toParamSchema (Proxy :: Proxy Int8)
    In an equation for ‘it’:
        it = encode $ toParamSchema (Proxy :: Proxy Int8)

This can be fixed by adding explicitly the (not very good looking) result type of toParamSchema:

>>> encode (toParamSchema (Proxy :: Proxy Integer) :: ParamSchema t Items)
"{\"type\":\"integer\"}"

With SwaggerItems, ambiguity disappears:

>>> encode $ toParamSchema (Proxy :: Proxy Integer)
"{\"type\":\"integer\"}"

SwaggerItems has a tiny disadvantage of allowing Items to be items type for Schema, but since it can always be converted naturally into Schema, it does not look bad. Besides, I don't expect library users to ever notice this or actually work with items directly.

@fizruk
Copy link
Member Author

fizruk commented Dec 14, 2015

@dmjio I would like to hear your opinion on this one before merging :)

@fizruk fizruk modified the milestone: v1.0 Dec 14, 2015
@dmjio
Copy link
Collaborator

dmjio commented Dec 14, 2015

@fizruk I'll look at it today ! apologies I've been traveling for work =)

On Mon, Dec 14, 2015 at 4:25 AM, Nickolay Kudasov notifications@github.com
wrote:

@dmjio https://github.com/dmjio I would like to hear your opinion on
this one before merging :)


Reply to this email directly or view it on GitHub
#24 (comment).

Cell: 1.630.740.8204

fizruk added a commit that referenced this pull request Dec 18, 2015
Introduce SwaggerItems GADT to simplify ParamSchema
@fizruk fizruk merged commit e1b45b1 into master Dec 18, 2015
@fizruk fizruk deleted the swagger-items branch December 18, 2015 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants