-
-
Notifications
You must be signed in to change notification settings - Fork 419
Fix Optional ReqBody' (wrap value into Maybe) #1347
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
base: master
Are you sure you want to change the base?
Fix Optional ReqBody' (wrap value into Maybe) #1347
Conversation
|
I’m not certain about these two patterns: (SFalse, STrue, False) -> return . either (const Nothing) (Just . Right)
(SFalse, SFalse, False) -> return . either (const Nothing) JustBut they seem to work as expected. It turned ouit that requests from the tests are having |
|
Hi! Is there something keeping this from going upstream? I was under the impression this was the default behavior (that one would construct a custom type with |
|
Yep', there is a conflict to be resolved in this PR. :) |
c766e36 to
838623a
Compare
|
@tchoutri I resolved the conflicts and rebased the change on top of latest master. Also I ran the tests locally with a successful result. Please have a look. |
|
Is there anything we can do to help maintainers move this PR forward? |
|
I somehow managed to miss the mention from 12 days ago. There could be a CI failure due to HsOpenSSL that I'm exploring in #1635, otherwise I don't have anything more to ask. :) |
|
@unclechu okay, I believe that you should bump the version of HsOpenSSL used, reading its changelog: https://flora.pm/packages/%40hackage/HsOpenSSL/0.11.7.5/changelog |
|
I'd like to see this PR merged - is there something I can do to help? I guess if I wanted to rebase it myself I'd have to open a separate PR? |
Closes #1346
Here is a small reproducible demonstration of the fix (requires Nix to run):
https://gist.github.com/unclechu/77ec314b8651c0c17ea4ac486fdb9804
Is resolving to
[Integer]in server serializer. This fix makes it becomeMaybe [Integer]instead (because it hasOptionalmode).This PR brings these changes:
OptionalReqBody'wrap it’s type intoMaybeNothingContent-Typebut without payload resolves toNothingContent-Typeheader is provided and payload is there it resolvesJustin case there were no parsing errorsRequiredReqBody'behaves as usual (not affected by this fix)