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

Add NoContentVerb #1219

Closed

Conversation

fierce-katie
Copy link
Contributor

Closes #1028.

The NoContentVerb endpoints always return status code 204 and empty body and it is reflected in the docs.

@fierce-katie
Copy link
Contributor Author

The only failing build on Travis CI is the one for ghc-8.0.2. It fails with

internal error in InstallPlan.completed
CallStack (from HasCallStack):
  error, called at ./Distribution/Client/InstallPlan.hs:262:25 in main:Distribution.Client.InstallPlan

I have not changed the CI configuration.

Probably related to haskell/cabal#6060

Copy link
Contributor

@phadej phadej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an opinion on this

Probably this is a right thing to do now.

#841 should make this unnecessary, but it's a holy grail which would fix everything, yet not founded still :)


I think that we should remove NoContent type if we go with this. ping @alpmestan

EDIT: above doesn't need to be done in this PR; also I'll figure what's wrong with CI myself

@@ -285,6 +298,15 @@ instance {-# OVERLAPPING #-}
where method = reflectMethod (Proxy :: Proxy method)
status = toEnum . fromInteger $ natVal (Proxy :: Proxy status)

instance {-# OVERLAPPABLE #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OVERLAPPABLE/OVERLAPPIG (here and elsewhere) are not required, are they?

@alpmestan
Copy link
Contributor

I like this patch, because users don't have to specify an arbitrary content type anymore.

@phadej phadej added this to the 0.17 milestone Sep 29, 2019
@phadej
Copy link
Contributor

phadej commented Sep 29, 2019

Thanks. Merged rebased version in #1228 (CI was green there).

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.

servant-docs: Incorrect docs generated for NoContent responses
3 participants