Skip to content

feat: Add isEnabled to Logs#1648

Merged
fuzzybinary merged 1 commit into
developfrom
jward/isEnabled
Sep 29, 2023
Merged

feat: Add isEnabled to Logs#1648
fuzzybinary merged 1 commit into
developfrom
jward/isEnabled

Conversation

@fuzzybinary

Copy link
Copy Markdown
Member

What does this PR do?

Added a static method to Logs for checking if the feature had been enabled in a core. This method is needed to properly setup Flutter when using attachToExisting.

refs: RUM-1066

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@fuzzybinary
fuzzybinary requested a review from a team as a code owner September 28, 2023 18:42
@fuzzybinary
fuzzybinary force-pushed the jward/isEnabled branch 2 times, most recently from 194afab to 0cfa80c Compare September 28, 2023 19:26
*/
@JvmOverloads
@JvmStatic
fun isEnabled(sdkCore: SdkCore = Datadog.getInstance()): Boolean {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems like implementation is different from iOS, where this method is public, but is still in the internal extension? Here we expose it directly, without any proxies.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I can make it internal but I based the decision based on the documentation on GlobalRumMonitor. I figured Android might have more of a need for such a method than iOS, but if you think not so much I can make the internal proxy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm ok with having it as public, just was curious about the consistency of public API with iOS.

@codecov-commenter

codecov-commenter commented Sep 29, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1648 (d7f5692) into develop (60e5c53) will increase coverage by 0.07%.
The diff coverage is 50.00%.

@@             Coverage Diff             @@
##           develop    #1648      +/-   ##
===========================================
+ Coverage    83.68%   83.76%   +0.07%     
===========================================
  Files          456      456              
  Lines        15690    15692       +2     
  Branches      2345     2346       +1     
===========================================
+ Hits         13130    13143      +13     
+ Misses        1931     1928       -3     
+ Partials       629      621       -8     
Files Coverage Δ
...gs/src/main/kotlin/com/datadog/android/log/Logs.kt 75.00% <50.00%> (-8.33%) ⬇️

... and 16 files with indirect coverage changes

Method is needed to properly setup Flutter when using `attachToExisting`.

refs: RUM-1066
@fuzzybinary
fuzzybinary merged commit c757420 into develop Sep 29, 2023
@fuzzybinary
fuzzybinary deleted the jward/isEnabled branch September 29, 2023 17:46
@xgouchet xgouchet added this to the 2.2.0 milestone Dec 13, 2023
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.

4 participants