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

Add View Hierarchy #2708

Merged
merged 39 commits into from
Feb 7, 2023
Merged

Add View Hierarchy #2708

merged 39 commits into from
Feb 7, 2023

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Dec 19, 2022 •
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Works both on iOS and Android but it can't be merged until stable releases of the native SDKs.

💡 Motivation and Context

💚 How did you test it?

sample app,

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2022 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 360.31 ms 380.96 ms 20.64 ms
Size 17.73 MiB 20.04 MiB 2.30 MiB

Baseline results on branch: 5.0.0

Startup times

Revision Plain With Sentry Diff
d091239+dirty 425.61 ms 447.11 ms 21.50 ms
d760caf+dirty 367.76 ms 388.06 ms 20.30 ms
b868b53+dirty 339.52 ms 369.84 ms 30.32 ms

App size

Revision Plain With Sentry Diff
d091239+dirty 17.73 MiB 19.95 MiB 2.22 MiB
d760caf+dirty 17.73 MiB 20.03 MiB 2.30 MiB
b868b53+dirty 17.73 MiB 19.95 MiB 2.22 MiB

@github-actions
Copy link
Contributor

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@krystofwoldrich krystofwoldrich changed the title View Hierarchy (iOS only) View Hierarchy Jan 11, 2023
@markushi markushi mentioned this pull request Jan 12, 2023
Closed
Base automatically changed from cocoa-v8 to 5.0.0 January 23, 2023 15:14
@krystofwoldrich
Copy link
Member Author

krystofwoldrich commented Jan 24, 2023 •
edited
Loading

I've simplified the view hierarchy function based on

@marandaneto
Copy link
Contributor

I've simplified the view hierarchy function based on

Let me know once this is ready to be reviewed again since there are conflicts to be solved yet + using the new Android API.

@krystofwoldrich
Copy link
Member Author

@marandaneto sentry-java is merged. This is ready for the final review.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 31, 2023 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1244.02 ms 1260.12 ms 16.10 ms
Size 2.36 MiB 2.82 MiB 462.38 KiB

Baseline results on branch: 5.0.0

Startup times

Revision Plain With Sentry Diff
d091239+dirty 1251.49 ms 1276.75 ms 25.26 ms
d760caf+dirty 1266.80 ms 1290.92 ms 24.12 ms
b868b53+dirty 1274.90 ms 1305.68 ms 30.78 ms

App size

Revision Plain With Sentry Diff
d091239+dirty 2.36 MiB 2.74 MiB 384.64 KiB
d760caf+dirty 2.36 MiB 2.81 MiB 462.03 KiB
b868b53+dirty 2.36 MiB 2.74 MiB 384.64 KiB

@krystofwoldrich krystofwoldrich mentioned this pull request Feb 1, 2023
Merged
5 tasks
marandaneto
marandaneto reviewed Feb 1, 2023
View reviewed changes
sample/src/App.tsx Outdated Show resolved Hide resolved
@krystofwoldrich krystofwoldrich mentioned this pull request Feb 2, 2023
Merged
7 tasks
@krystofwoldrich
Copy link
Member Author

One last thing

marandaneto
marandaneto approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM after upgrading the Android SDK with the latest changes (dupe of VH attachments)

@krystofwoldrich
Copy link
Member Author

Waiting for https://github.com/getsentry/sentry-java release, likely 6.13.1.

@krystofwoldrich krystofwoldrich mentioned this pull request Feb 3, 2023
Merged
@krystofwoldrich krystofwoldrich changed the title View Hierarchy Add View Hierarchy Feb 7, 2023
@krystofwoldrich krystofwoldrich merged commit a71ab71 into 5.0.0 Feb 7, 2023
@krystofwoldrich krystofwoldrich deleted the view-hierarchy branch February 7, 2023 14:48
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Add View Hierarchy ([#2708](https://github.com/getsentry/sentry-react-native/pull/2708))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against cd61177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marandaneto marandaneto marandaneto 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.
3 participants
@krystofwoldrich @marandaneto @web-flow