-
Notifications
You must be signed in to change notification settings - Fork 15
Deprecate all SafeTrace/SafeTraceFunctions APIs #83
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
base: main
Are you sure you want to change the base?
Conversation
69392b7 to
e0e00fc
Compare
| androidx.tracing.Trace.beginSection(label.take(MAX_LABEL_LENGTH)) | ||
| } | ||
|
|
||
| @Deprecated("Call androidx.tracing.Trace.beginSection instead", ReplaceWith("beginSection", "androidx.tracing.Trace.beginSection")) |
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.
androidx.tracing.Trace.beginSection doesn't support a lambda, but we could provide an extension function for that.
| /** | ||
| * @see [logSection] | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.beginSection/endSection instead") |
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.
logSection doesn't exist in Jetpack, but we could provide an extension function androidx.tracing.Trace.logSection to do this.
| /** | ||
| * @see androidx.tracing.Trace.beginAsyncSection | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.beginAsyncSection instead", ReplaceWith("beginAsyncSection", "androidx.tracing.Trace.beginAsyncSection")) |
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.
androidx.tracing.Trace.beginAsyncSection doesn't support passing a lambda, but we could provide an extension function for that.
| enableMainThreadMessageTracing() | ||
| } | ||
|
|
||
| fun enableMainThreadMessageTracing() { |
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.
Since we're deprecating forceEnableAppTracing, we need to expose a way for callers to invoke enableMainThreadMessageTracing directly.
7830ed7 to
5dd78bf
Compare
5dd78bf to
f3ec685
Compare
1125edb to
a81398e
Compare
a81398e to
21b7a35
Compare
| <manifest xmlns:android="http://schemas.android.com/apk/res/android"> | ||
|
|
||
| <application> | ||
| <!-- A receiver that makes the build shell profileable on broadcast, disabled by default. --> |
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.
These are unused.
| val isCurrentlyTracing: Boolean | ||
| get() = androidx.tracing.Trace.isEnabled() | ||
|
|
||
| @Deprecated("Use forceShellProfileable instead", ReplaceWith("forceShellProfileable()")) |
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.
This was unused. Rather than keep a deprecated version of it, I think it makes more sense to remove it entirely.
| fun init(application: Application) { | ||
| this.application = application | ||
| enableMainThreadMessageTracing() | ||
| } |
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.
Do we still need the application field here?
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 need it until I remove all callers of SafeTrace.isTraceable. Then I'll remove it.
| } | ||
|
|
||
| fun enableMainThreadMessageTracing() { | ||
| SafeTraceMainThreadMessages.enableMainThreadMessageTracing() |
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.
we probably ought to make a public API for this, but ideally something cohesive not a random function of a SafeTrace object that's not about safe traces anymore
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.
How about I move SafeTraceMainThreadMessages out of the internal package and make it public?
| "PAPA-cancel:timeout" | ||
| } | ||
| Trace.beginSection("PAPA-cancel:timeout") | ||
| Trace.endSection() |
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.
why not use the lambda version with {} ?
or at least bring back logSection as helper function here?
No description provided.