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.

Sign up for GitHub

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

Jump to bottom

feat(profiling): Support ingesting mixed android/js profiles for react-native #375

Merged
merged 4 commits into from
Dec 19, 2023
Merged

feat(profiling): Support ingesting mixed android/js profiles for react-native #375

merged 4 commits into from
Dec 19, 2023

Conversation

viglia
Copy link
Contributor

@viglia viglia commented Dec 7, 2023

This is related to the Sentry PR

@viglia viglia requested a review from a team as a code owner December 7, 2023 16:18
@kahest kahest mentioned this pull request Dec 18, 2023
Closed
phacops
phacops reviewed Dec 18, 2023
View reviewed changes
Copy link
Contributor

@phacops phacops left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Don't we need a way to render the profile as part of the Android speedscope format? I guess we can leave it for another PR though, just thinking of the work ahead to fully "support" this.

Maybe tweak the changelog to be clear about only storing it?

@viglia
Copy link
Contributor Author

viglia commented Dec 19, 2023

Don't we need a way to render the profile as part of the Android speedscope format? I guess we can leave it for another PR though, just thinking of the work ahead to fully "support" this.

Maybe tweak the changelog to be clear about only storing it?

yes, that is correct. I split it in 2 different PRs. This is only about ingesting those profiles, not rendering.
I'll update the Changelog to make it clearer.

@viglia viglia merged commit 03b2f69 into main Dec 19, 2023
11 checks passed
@viglia viglia deleted the viglia/feat/support-js_profile-in-android branch December 19, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phacops phacops phacops approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viglia @phacops