feat: Add http4s client and server modules with simple tests #18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jakubjanecek
requested review from
augi,
hanny24,
jendakol,
sideeffffect and
tomasherman
tomasherman
previously requested changes
Sep 25, 2019
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerConfig.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerConfig.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/CorrelationIdMiddleware.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/CorrelationIdMiddleware.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/CorrelationIdMiddleware.scala
Outdated
Show resolved
Hide resolved
hanny24
reviewed
Sep 25, 2019
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientModule.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sRouting.scala
Outdated
Show resolved
Hide resolved
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientModule.scala
Outdated
Show resolved
Hide resolved
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientModule.scala
Outdated
Show resolved
Hide resolved
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientModule.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sRouting.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerConfig.scala
Outdated
Show resolved
Hide resolved
augi
requested changes
Sep 26, 2019
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientModule.scala
Outdated
Show resolved
Hide resolved
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientConfig.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerConfig.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerConfig.scala
Outdated
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerConfig.scala
Show resolved
Hide resolved
http4s-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sServerModule.scala
Outdated
Show resolved
Hide resolved
sideeffffect
suggested changes
Sep 26, 2019
Contributor
sideeffffect
left a comment
sideeffffect
left a comment
There was a problem hiding this comment.
I would avoid creating wrapper classes which don't add any value, like class Http4sClientModule
http4s-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sClientModule.scala
Outdated
Show resolved
Hide resolved
Contributor
|
@jakubjanecek it seems to me that you intended the modules/config case classes not just for any http4s server/client, but specifically for the Blaze flavours. So why not reflect that in the name of the objects, like |
Collaborator
Author
|
@sideeffffect I agree. After the discussions here I see that I made the mistake of making it generic http4s thing but it's actually only one of the implementations which is |
jakubjanecek
requested review from
augi,
hanny24,
sideeffffect and
tomasherman
Collaborator
Author
|
I've tried to address all your comments somehow. Please have another look. Thanks. |
http4s-blaze-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeClient.scala
Show resolved
Hide resolved
http4s-blaze-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeClient.scala
Outdated
Show resolved
Hide resolved
...4s-blaze-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeClientConfig.scala
Show resolved
Hide resolved
...4s-blaze-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeClientConfig.scala
Outdated
Show resolved
Hide resolved
...4s-blaze-server/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeServerConfig.scala
Show resolved
Hide resolved
http4s-blaze-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeClient.scala
Show resolved
Hide resolved
...4s-blaze-server/src/main/scala/com/avast/server/toolkit/http4s/CorrelationIdMiddleware.scala
Outdated
Show resolved
Hide resolved
jakubjanecek
added
documentation
Collaborator
Author
|
Everyone I think this is ready for the last round of review and possibly merging. |
jakubjanecek
force-pushed
the
feat/http4s
branch
from
9cd540f to
13153c0
Compare
hanny24
approved these changes
Sep 30, 2019
augi
approved these changes
Sep 30, 2019
example/src/main/scala/com/avast/server/toolkit/example/Test.scala
Outdated
Show resolved
Hide resolved
...4s-blaze-client/src/main/scala/com/avast/server/toolkit/http4s/Http4sBlazeClientConfig.scala
Show resolved
Hide resolved
...rver/src/main/scala/com/avast/server/toolkit/http4s/middleware/CorrelationIdMiddleware.scala
Show resolved
Hide resolved
http4s modules renamed to http4s-blaze-* to be more precise refactor: CorrelationIdMiddleware made more general refactor: Http4sRouting made more general
refactor: PR review changes docs: Add http4s docs and tweak mdoc configuration
jakubjanecek
force-pushed
the
feat/http4s
branch
from
7169daf to
ade1f62
Compare
sideeffffect
approved these changes
Sep 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is not fully finished but I thought I'd put it up for review earlier. Docs and examples are missing, will be added before merging.