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

Additional log implementation refactor #1806

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Jan 10, 2025

Goal

Builds on #1805 by continuing a refactor of the LogService to simplify the parameters passed in. This should make it easier for when we add the file attachment functionality.

Testing

Relied on existing test coverage.

@fractalwrench fractalwrench force-pushed the refactor-log-impl branch 4 times, most recently from 74bc3c4 to 94f7f51 Compare January 13, 2025 10:10
@fractalwrench fractalwrench force-pushed the refactor-log-impl-2 branch 2 times, most recently from 0d782b2 to 9baf117 Compare January 13, 2025 11:11
@fractalwrench fractalwrench changed the title poc: continue refactoring log implementation Additional log implementation refactor Jan 13, 2025
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 82.35294% with 15 lines in your changes missing coverage. Please review.

Project coverage is 85.44%. Comparing base (c6c297a) to head (ea9d7fd).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...l/api/delegate/ReactNativeInternalInterfaceImpl.kt 0.00% 5 Missing ⚠️
...ernal/api/delegate/EmbraceInternalInterfaceImpl.kt 0.00% 4 Missing ⚠️
.../java/io/embrace/android/embracesdk/EmbraceImpl.kt 86.66% 2 Missing ⚠️
...ernal/api/delegate/FlutterInternalInterfaceImpl.kt 80.00% 0 Missing and 2 partials ⚠️
...mbracesdk/internal/api/delegate/LogsApiDelegate.kt 94.59% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1806      +/-   ##
==========================================
- Coverage   85.45%   85.44%   -0.02%     
==========================================
  Files         473      473              
  Lines       11051    11015      -36     
  Branches     1650     1648       -2     
==========================================
- Hits         9444     9412      -32     
  Misses        870      870              
+ Partials      737      733       -4     
Files with missing lines Coverage Δ
...oid/embracesdk/internal/injection/LogModuleImpl.kt 91.42% <100.00%> (-0.24%) ⬇️
...roid/embracesdk/internal/logs/EmbraceLogService.kt 98.07% <100.00%> (+5.53%) ⬆️
...ace/android/embracesdk/internal/logs/LogService.kt 100.00% <100.00%> (ø)
...nternal/api/delegate/UnityInternalInterfaceImpl.kt 44.23% <100.00%> (-3.99%) ⬇️
.../java/io/embrace/android/embracesdk/EmbraceImpl.kt 75.29% <86.66%> (-1.82%) ⬇️
...ernal/api/delegate/FlutterInternalInterfaceImpl.kt 85.71% <80.00%> (-7.15%) ⬇️
...mbracesdk/internal/api/delegate/LogsApiDelegate.kt 85.07% <94.59%> (-0.26%) ⬇️
...ernal/api/delegate/EmbraceInternalInterfaceImpl.kt 78.26% <0.00%> (-6.68%) ⬇️
...l/api/delegate/ReactNativeInternalInterfaceImpl.kt 71.05% <0.00%> (+5.19%) ⬆️

... and 4 files with indirect coverage changes

@fractalwrench fractalwrench marked this pull request as ready for review January 13, 2025 11:26
@fractalwrench fractalwrench requested a review from a team as a code owner January 13, 2025 11:26
Base automatically changed from refactor-log-impl to main January 13, 2025 12:57
@fractalwrench fractalwrench changed the base branch from main to file-attachment January 13, 2025 13:05
Base automatically changed from file-attachment to main January 13, 2025 13:31
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

* @param context the context of the exception
* @param library the library of the exception
* @param exceptionName the exception name of a Throwable is it is present
* @param exceptionMessage the exception message of a Throwable is it is present
*/
@Suppress("LongParameterList")
Copy link
Collaborator

Choose a reason for hiding this comment

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

After refactoring we can probably try to remove all the suppresses in these classes - I hope none of the remaining ones are above 10

Copy link
Collaborator

@bidetofevil bidetofevil left a comment

Choose a reason for hiding this comment

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

LGTM

@fractalwrench fractalwrench merged commit 425639c into main Jan 14, 2025
7 checks passed
@fractalwrench fractalwrench deleted the refactor-log-impl-2 branch January 14, 2025 09:28
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.

2 participants