-
Notifications
You must be signed in to change notification settings - Fork 62
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
RUM-6286: replacing joinToString
when it possible, refactor a bit
#2456
RUM-6286: replacing joinToString
when it possible, refactor a bit
#2456
Conversation
dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/StringBuilderExtKtTest.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/StringBuilderExtKtTest.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/test/kotlin/com/datadog/android/utils/PerformanceTest.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/test/kotlin/com/datadog/android/utils/PerformanceTest.kt
Outdated
Show resolved
Hide resolved
ece50e3
to
39353fd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2456 +/- ##
===========================================
- Coverage 69.99% 69.95% -0.04%
===========================================
Files 785 786 +1
Lines 29278 29279 +1
Branches 4884 4886 +2
===========================================
- Hits 20491 20480 -11
- Misses 7430 7441 +11
- Partials 1357 1358 +1
|
77c4ba7
to
b03562c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left few comments/suggestions. The only blocker for me is having new extension method in the core
module instead of internal
.
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/StringBuilderExt.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/StringBuilderExt.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/StringBuilderExt.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/StringBuilderExt.kt
Outdated
Show resolved
Hide resolved
...ore/src/test/kotlin/com/datadog/android/utils/JointToStringVsStringBuilderPerformanceTest.kt
Outdated
Show resolved
Hide resolved
...ore/src/test/kotlin/com/datadog/android/utils/JointToStringVsStringBuilderPerformanceTest.kt
Outdated
Show resolved
Hide resolved
...ore/src/test/kotlin/com/datadog/android/utils/JointToStringVsStringBuilderPerformanceTest.kt
Outdated
Show resolved
Hide resolved
...ore/src/test/kotlin/com/datadog/android/utils/JointToStringVsStringBuilderPerformanceTest.kt
Outdated
Show resolved
Hide resolved
b03562c
to
55518a7
Compare
92867f7
to
f4159a6
Compare
dd-sdk-android-internal/src/test/java/com/datadog/internal/utils/StringBuilderExtKtTest.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-internal/src/test/java/com/datadog/internal/utils/StringBuilderExtKtTest.kt
Outdated
Show resolved
Hide resolved
2255594
to
2261b18
Compare
2261b18
to
b2a6eae
Compare
What does this PR do?
Here we're trying to reduce usages of joinToString method inside the project, replacing it with more performant string builder
Motivation
Using
joinToString
may cause a lot of memory and cpu consumption which we trying to avoid, minimizing impact of sdk to user's codeAdditional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)