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

Do not ignore require cycle logs for RN >= 0.70 #4214

Merged
merged 4 commits into from
Oct 31, 2024
Merged

Do not ignore require cycle logs for RN >= 0.70 #4214

merged 4 commits into from
Oct 31, 2024

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Oct 29, 2024 •
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

This PR skips ignoring require cycle logs for RN >= 0.70

💡 Motivation and Context

fixes #3484

💚 How did you test it?

Manual, CI

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

@antonis antonis changed the title Do not ignore require cycle logs for RN > 0.70 Do not ignore require cycle logs for RN >= 0.70 Oct 29, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 29, 2024 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 490.51 ms 479.42 ms -11.09 ms
Size 17.74 MiB 20.08 MiB 2.34 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f06c879 408.41 ms 424.54 ms 16.13 ms
0ebca77 414.93 ms 444.49 ms 29.56 ms
e2b64fe 316.88 ms 330.23 ms 13.35 ms
148f924 492.65 ms 500.28 ms 7.63 ms
d7401ac+dirty 375.20 ms 383.51 ms 8.31 ms
34aba08 328.10 ms 342.84 ms 14.74 ms
62a750b 395.96 ms 423.36 ms 27.41 ms
1c65324 426.37 ms 460.36 ms 33.99 ms
86d6d2c+dirty 332.90 ms 352.45 ms 19.55 ms
e73d82f 475.82 ms 506.55 ms 30.73 ms

App size

Revision Plain With Sentry Diff
f06c879 17.73 MiB 19.85 MiB 2.12 MiB
0ebca77 17.73 MiB 19.95 MiB 2.21 MiB
e2b64fe 17.73 MiB 19.80 MiB 2.07 MiB
148f924 17.73 MiB 19.94 MiB 2.21 MiB
d7401ac+dirty 17.73 MiB 19.75 MiB 2.02 MiB
34aba08 17.73 MiB 19.80 MiB 2.07 MiB
62a750b 17.73 MiB 19.93 MiB 2.20 MiB
1c65324 17.73 MiB 19.95 MiB 2.21 MiB
86d6d2c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
e73d82f 17.73 MiB 20.07 MiB 2.33 MiB

Previous results on branch: antonis/3484-ignorerequirecyclewarnings

Startup times

Revision Plain With Sentry Diff
12f7ee0 467.08 ms 471.08 ms 4.00 ms

App size

Revision Plain With Sentry Diff
12f7ee0 17.74 MiB 20.08 MiB 2.34 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 29, 2024 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1225.67 ms 1223.94 ms -1.73 ms
Size 2.36 MiB 3.08 MiB 736.81 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4297324+dirty 1230.27 ms 1232.53 ms 2.27 ms
e73f4ed+dirty 1243.27 ms 1244.52 ms 1.25 ms
3ffcddd+dirty 1244.47 ms 1264.14 ms 19.67 ms
a989877+dirty 1228.56 ms 1227.71 ms -0.85 ms
27ef4ee+dirty 1293.52 ms 1296.08 ms 2.56 ms
fe13591+dirty 1208.25 ms 1219.53 ms 11.28 ms
e540498+dirty 1220.61 ms 1212.93 ms -7.68 ms
5a22220+dirty 1209.49 ms 1220.94 ms 11.45 ms
e5c9b8b+dirty 1258.57 ms 1267.32 ms 8.75 ms
f06c879+dirty 1252.64 ms 1259.66 ms 7.02 ms

App size

Revision Plain With Sentry Diff
4297324+dirty 2.36 MiB 3.08 MiB 735.61 KiB
e73f4ed+dirty 2.36 MiB 2.82 MiB 469.44 KiB
3ffcddd+dirty 2.36 MiB 2.84 MiB 489.60 KiB
a989877+dirty 2.36 MiB 3.10 MiB 752.40 KiB
27ef4ee+dirty 2.36 MiB 2.85 MiB 500.03 KiB
fe13591+dirty 2.36 MiB 3.10 MiB 752.40 KiB
e540498+dirty 2.36 MiB 3.14 MiB 793.34 KiB
5a22220+dirty 2.36 MiB 2.92 MiB 570.21 KiB
e5c9b8b+dirty 2.36 MiB 2.87 MiB 520.43 KiB
f06c879+dirty 2.36 MiB 2.88 MiB 530.42 KiB

Previous results on branch: antonis/3484-ignorerequirecyclewarnings

Startup times

Revision Plain With Sentry Diff
12f7ee0+dirty 1225.88 ms 1232.14 ms 6.27 ms

App size

Revision Plain With Sentry Diff
12f7ee0+dirty 2.36 MiB 3.08 MiB 735.65 KiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 29, 2024 •
edited
Loading

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1244.90 ms 1238.77 ms -6.12 ms
Size 2.92 MiB 3.64 MiB 742.64 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9c48b2c+dirty 1253.39 ms 1256.30 ms 2.91 ms
22e31b6+dirty 1276.55 ms 1278.12 ms 1.57 ms
9433f35+dirty 1232.24 ms 1232.74 ms 0.50 ms
5a22220+dirty 1246.18 ms 1249.61 ms 3.43 ms
76d1baf+dirty 1245.00 ms 1257.76 ms 12.76 ms
5446992+dirty 1249.94 ms 1254.80 ms 4.86 ms
dadc233+dirty 1266.52 ms 1282.55 ms 16.03 ms
abb7058+dirty 1260.28 ms 1266.56 ms 6.28 ms
34aba08+dirty 1268.58 ms 1276.80 ms 8.22 ms
ad6c299+dirty 1248.50 ms 1248.88 ms 0.38 ms

App size

Revision Plain With Sentry Diff
9c48b2c+dirty 2.92 MiB 3.41 MiB 499.97 KiB
22e31b6+dirty 2.92 MiB 3.43 MiB 524.74 KiB
9433f35+dirty 2.92 MiB 3.41 MiB 503.55 KiB
5a22220+dirty 2.92 MiB 3.48 MiB 575.81 KiB
76d1baf+dirty 2.92 MiB 3.38 MiB 475.74 KiB
5446992+dirty 2.92 MiB 3.44 MiB 535.26 KiB
dadc233+dirty 2.92 MiB 3.40 MiB 492.53 KiB
abb7058+dirty 2.92 MiB 3.43 MiB 524.53 KiB
34aba08+dirty 2.92 MiB 3.41 MiB 499.03 KiB
ad6c299+dirty 2.92 MiB 3.40 MiB 494.12 KiB

Previous results on branch: antonis/3484-ignorerequirecyclewarnings

Startup times

Revision Plain With Sentry Diff
12f7ee0+dirty 1241.43 ms 1234.15 ms -7.28 ms

App size

Revision Plain With Sentry Diff
12f7ee0+dirty 2.92 MiB 3.64 MiB 741.28 KiB

@antonis antonis marked this pull request as ready for review October 29, 2024 15:49
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 29, 2024 •
edited
Loading

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 377.28 ms 413.45 ms 36.17 ms
Size 7.15 MiB 8.35 MiB 1.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b1e8712+dirty 322.55 ms 331.84 ms 9.29 ms
70e6261+dirty 395.08 ms 408.12 ms 13.04 ms
2ec71da+dirty 375.64 ms 431.59 ms 55.95 ms
c2a4e9b+dirty 392.94 ms 474.55 ms 81.61 ms
80b2ce3+dirty 271.29 ms 316.47 ms 45.18 ms
d7401ac+dirty 373.98 ms 394.08 ms 20.10 ms
31fcca2+dirty 366.64 ms 395.78 ms 29.14 ms
4297324+dirty 385.33 ms 435.68 ms 50.35 ms
e2b64fe+dirty 258.82 ms 304.26 ms 45.44 ms
86d6d2c+dirty 267.21 ms 325.24 ms 58.04 ms

App size

Revision Plain With Sentry Diff
b1e8712+dirty 7.15 MiB 8.04 MiB 912.27 KiB
70e6261+dirty 7.15 MiB 8.21 MiB 1.07 MiB
2ec71da+dirty 7.15 MiB 8.38 MiB 1.23 MiB
c2a4e9b+dirty 7.15 MiB 8.34 MiB 1.19 MiB
80b2ce3+dirty 7.15 MiB 8.04 MiB 911.02 KiB
d7401ac+dirty 7.15 MiB 8.04 MiB 910.85 KiB
31fcca2+dirty 7.15 MiB 8.18 MiB 1.03 MiB
4297324+dirty 7.15 MiB 8.35 MiB 1.20 MiB
e2b64fe+dirty 7.15 MiB 8.07 MiB 947.16 KiB
86d6d2c+dirty 7.15 MiB 8.09 MiB 962.69 KiB

Previous results on branch: antonis/3484-ignorerequirecyclewarnings

Startup times

Revision Plain With Sentry Diff
12f7ee0+dirty 402.11 ms 433.40 ms 31.29 ms

App size

Revision Plain With Sentry Diff
12f7ee0+dirty 7.15 MiB 8.35 MiB 1.20 MiB

lucas-zimerman
lucas-zimerman reviewed Oct 30, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
lucas-zimerman
lucas-zimerman approved these changes Oct 30, 2024
View reviewed changes
Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

Left a suggestion on the changelog, after resolving it, LGTM!

antonis and others added 2 commits October 30, 2024 15:30
@antonis @lucas-zimerman
Update changelog
cc78b05 
Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
@antonis
Merge branch 'main' into antonis/3484-ignorerequirecyclewarnings
11821ed 
# Conflicts:
#	CHANGELOG.md
@antonis antonis merged commit 1faf8e3 into main Oct 31, 2024
60 checks passed
@antonis antonis deleted the antonis/3484-ignorerequirecyclewarnings branch October 31, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucas-zimerman lucas-zimerman lucas-zimerman approved these changes

@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich krystofwoldrich is a code owner

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

Successfully merging this pull request may close these issues.

Sentry.init() is disabling require cycle warnings for the entire app it's integrated in
2 participants
@antonis @lucas-zimerman