Conversation
kdabir
left a comment
There was a problem hiding this comment.
Thanks for taking time to contribute. At present we are not in position to merge this request because of two reasons. 1. it used mutable variable 2. it does not include test cases.
src/main/kotlin/ktee/KTee.kt
Outdated
| import ktee.KTee.Companion.debug | ||
| import org.slf4j.Logger | ||
|
|
||
| class KTee { companion object { var debug = true } } |
There was a problem hiding this comment.
we actively avoid using var in our codebase. This becomes a global variable which is not good in server/multithreaded/concurrent envs.
There was a problem hiding this comment.
I've made some changes. The variable can now be set only once.
If you are concerned about thread safety, @volatile can also be used instead.
Other than that, I fail to see how it's possible to have a toggle without being a var.
|
Fixed the second issue. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
Can we look at alternative like a No-Op interface just like slf4j. That way dependency can be swapped at build/compile time without having to change code. in no-op impl all the methods essentially do not output anything. We can produce two artifacts |
|
Kudos, SonarCloud Quality Gate passed! |
|
@kdabir, coming from the "Android world" where build types and flavors are a thing in Gradle, I can't be so sure I implemented the No-Op artifact publication in the best way possible. Please, let me know if there's something that can be improved. |
|
apologies for the delay in responding. The changes are looking good. For no-op project, we could go with a multi-module approach rather than additional source set. However, I don't necessarily see problem with current approach too. Let me review this and provide more specific feedback. Thanks for incorporating the feedback. |
|
Of course, I'm up for giving a try to any better solution. Could you please give me more info on how to approach a multi-module solution and what are the advantages compared to the current? |
# Conflicts: # build.gradle
|
Kudos, SonarCloud Quality Gate passed! |
|
If someone is looking for this feature, I released a modified version here. Using a noop implementation in production does make reverse engineering slightly harder, thus I recommend it unconditionally in any non server-side code. |








Edit:
With this PR, two artifacts will be published,
kteeandktee-noop.As the name suggests,
ktee-noophas no internal functionality and it can be used in production environment with no overhead.PS. I think this was suggested by someone in Reddit.