WIP #2074
WIP #2074
Conversation
jdegoes
commented
Mar 30, 2023
•
jdegoes
commented
- Polish ZClient
- Polish URL
- Polish Http
- Polish Handler
jdegoes
requested review from
amitksingh1490,
vigoo and
adamgfraser
as code owners
|
|
||
| def toForm: Form = Form.fromQueryParams(self) | ||
|
|
||
| def toMap: Map[String, Chunk[String]] = map |
There was a problem hiding this comment.
Looks a bit confusing that
- we are extending
Map[String, Chunk[String]] - also have a public field
map: Map[String, Chunk[String]] - and a
toMap: Map[String, Chunk[String]]
and self is not the same map as the "inner map"; one could ask what is the difference (apart from having extra methods on QueryParams), are all map operations equivalent to regardless when you call toMap, etc.
| @@ -28,60 +28,43 @@ import zio.http.socket.SocketApp | |||
| import java.net.{InetSocketAddress, URI} // scalafix:ok; | |||
|
|
|||
| trait ZClient[-Env, -In, +Err, +Out] extends HeaderOps[ZClient[Env, In, Err, Out]] { self => | |||
There was a problem hiding this comment.
Now that I moved config to ZClient.Config, I was wondering it would look better to have Client as the "main" companion object, hand have the layers and config class in that instead of ZClient. Would be more symmetric to Server.
There was a problem hiding this comment.
How about val Client = ZClient or is that too ZIO 1.0-ish?
There was a problem hiding this comment.
I think that's the best we came up with when using this more polymorphic variant pattern.
There was a problem hiding this comment.
We already have that. I proposed to switch them and make Client the "real" object, because right now in type signatures you have to use ZClient.Config
There was a problem hiding this comment.
Don't we just need a type alias? type Client = ZClient.type
There was a problem hiding this comment.
We could do trait Client[...] extends Client.Poly[...]
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #2074 +/- ##
==========================================
+ Coverage 66.66% 66.79% +0.12%
==========================================
Files 140 140
Lines 5817 5815 -2
Branches 226 232 +6
==========================================
+ Hits 3878 3884 +6
+ Misses 1939 1931 -8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
@vigoo Appreciate a review if you get a chance, would like to split this up since it's already so big. |
