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

removed eager ext #120 #131

Merged
merged 1 commit into from
Jul 16, 2019
Merged

Conversation

rybalkinsd
Copy link
Owner

No description provided.

@codecov
Copy link

codecov bot commented Jul 15, 2019

Codecov Report

Merging #131 into feature/128-multimodule will decrease coverage by 0.22%.
The diff coverage is 100%.

Impacted file tree graph

@@                      Coverage Diff                      @@
##             feature/128-multimodule     #131      +/-   ##
=============================================================
- Coverage                      90.51%   90.29%   -0.23%     
  Complexity                       119      119              
=============================================================
  Files                             41       41              
  Lines                            369      340      -29     
  Branches                          44       43       -1     
=============================================================
- Hits                             334      307      -27     
+ Misses                            10        9       -1     
+ Partials                          25       24       -1
Impacted Files Coverage Δ Complexity Δ
...tlin/io/github/rybalkinsd/kohttp/ext/HeadersExt.kt 100% <ø> (ø) 0 <0> (ø) ⬇️
...lin/io/github/rybalkinsd/kohttp/ext/ResponseExt.kt 0% <ø> (-87.1%) 0 <0> (ø)
...balkinsd/kohttp/interceptors/LoggingInterceptor.kt 100% <100%> (ø) 3 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15c0501...12667db. Read the comment docs.

@rybalkinsd rybalkinsd added this to the 0.11.0 milestone Jul 16, 2019
@@ -17,3 +17,5 @@ fun Headers.asSequence(): Sequence<Header> = Sequence {
}
}
}

typealias Header = Pair<String, String>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the problem to use old

data class Headers(...)

Even it can be destructed (watch next comment)

@@ -35,3 +35,9 @@ class HeadersExtKtTest {
}
}
}

val Header.name
get() = this.first
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use plain data class (as was before) we can omit such extensions

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created #134 to implement this case

@@ -24,7 +24,7 @@ class LoggingInterceptor(private val log: (String) -> Unit = ::println) : Interc
return chain.proceed(request).also { response ->
log("${request.method()} ${response.code()} - ${System.currentTimeMillis() - startTime}ms ${request.url()}")

request.headers().asSequence().forEach { log("${it.name}: ${it.value}") }
request.headers().asSequence().forEach { (name, value) ->log("$name: $value") }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data class also can be destructed, it is not necessary to make it alias for 'Pair<String, String>'

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason for it is Header data class is also removed in this PR

@rybalkinsd rybalkinsd merged commit 18f6ac5 into feature/128-multimodule Jul 16, 2019
@rybalkinsd rybalkinsd deleted the feature/120-remove-eager branch August 4, 2019 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants