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

Upgrade http4s to version 0.20.0 #275

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@mayboroda
Copy link

commented May 2, 2019

Upgrade of http4s library to current stable version 0.20.0

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.
@blast-hardcheese
Copy link
Collaborator

left a comment

Thank you for the work so far!

mayboroda and others added some commits May 4, 2019

@mayboroda

This comment has been minimized.

Copy link
Author

commented May 13, 2019

@blast-hardcheese I guess we resolved all the issue, may be we can merge this?

@@ -233,7 +236,7 @@ object Http4sServerGenerator {
None,
Some(
(
q"urlForm.values.get(${arg.argName.toLit}).map(_.map(Json.fromString(_).as[$tpe]).sequence)",
q"urlForm.values.get(${arg.argName.toLit}).flatMap(_.toList).traverse(Json.fromString(_).as[$tpe])",

This comment has been minimized.

Copy link
@blast-hardcheese

blast-hardcheese May 13, 2019

Collaborator

I'm very wary of .values.get("foo").flatMap(_.toList), I haven't had time to test it to be certain, but my intuition is that urlForm.values.get(...) would be Map, which would mean get would return Option[_], which would mean flatMap(_.toList) would fail.

If you're available to hit this codepath or otherwise test this construct in the REPL, I'd appreciate it. I'd like to get this out, I'd just like to get it out without breaking functionality (as I expect you can appreciate).

Please pardon the delay around this, I should have been more communicative.

This comment has been minimized.

Copy link
@mayboroda

mayboroda May 14, 2019

Author

Deal, I'll try to do my best to give to a REPL example and a fix if needed :)

@blast-hardcheese blast-hardcheese force-pushed the twilio:master branch from 6a41c95 to 3f0f265 May 15, 2019

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.