Upgrade to elm http2 - Githubissues

torrefatto commented 3 years ago

Hi! As we discussed earlier, here is a PR to upgrade to elm/http >= 2.0.0 and to address some of the concerns expressed in #42

In particular:

as in #42, the various send* functions now emit an Cmd msg
we expose some building block functions to allow the user to build it's graphql request via Http.request method from elm/http (for example graphQLBody and graphQLexpect)
we created a new Result type, that is a sort of three-state type, that carries the success or failure state without the need to double examine the result to get if the error is http-related or graphql-related
we tackled the issue of the error decoding, i.e. provide the graphql errors also if the decoding of the response succeeds (defaulting to an empty error list if everything goes smoothly)

What is missing is to update the documentation (indeed, elm make --doc=out.json fails). Let us know what do you think of the changes we've done so far and we will gladly update also the docs :blush:

jamesmacaulay commented 3 years ago

Thanks for this! I haven't had time to do a real review yet but from what I've seen it looks very good. I will try to complete the review this week.

Since this PR requires the other Travis PR for CI to work, perhaps it would be best to include the Travis config change in this PR? Otherwise I could merge the Travis PR into master and then you could rebase this one onto new master, is that what you had in mind?

torrefatto commented 3 years ago

Thanks for this! I haven't had time to do a real review yet but from what I've seen it looks very good. I will try to complete the review this week.

Great!

Since this PR requires the other Travis PR for CI to work, perhaps it would be best to include the Travis config change in this PR? Otherwise I could merge the Travis PR into master and then you could rebase this one onto new master, is that what you had in mind?

Sure, I was trying to do it in #44 with no success :sweat_smile:. When the only step failing will be the elm-format one, I will merge it here (sorry for opening #44, but I need access to travis).

jamesmacaulay / elm-graphql

Upgrade to elm http2 #43