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

test(java): Add PMD static code analysis for Java files #4183

Merged
merged 4 commits into from
Oct 16, 2024
Merged

test(java): Add PMD static code analysis for Java files #4183

merged 4 commits into from
Oct 16, 2024

Conversation

krystofwoldrich
Copy link
Member

📢 Type of change

  • Enhancement
  • Refactoring

📜 Description

Execute PMD (https://pmd.github.io/) static code analysis for Java code in the React Native repository.

The performance-tests apps use the ignore command to avoid changing the default RN template. But PMD is executed for them as well in case we change the apps in the future.

💡 Motivation and Context

💚 How did you test it?

ci, locally

📝 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

#skip-changelog

@github-actions GitHub Actions
Copy link
Contributor

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

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1236.39 ms 1234.80 ms -1.59 ms
Size 2.92 MiB 3.66 MiB 757.70 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
abb7058+dirty 1260.28 ms 1266.56 ms 6.28 ms
e540498+dirty 1254.92 ms 1247.21 ms -7.71 ms
d43a46b+dirty 1223.31 ms 1230.92 ms 7.61 ms
0677344+dirty 1252.52 ms 1254.08 ms 1.56 ms
148f924+dirty 1220.72 ms 1221.30 ms 0.58 ms
700cbf4+dirty 1233.96 ms 1228.27 ms -5.69 ms
22e31b6+dirty 1276.55 ms 1278.12 ms 1.57 ms
5bb8d5f+dirty 1215.04 ms 1217.52 ms 2.48 ms
e73d82f+dirty 1231.20 ms 1228.81 ms -2.40 ms
70e6261+dirty 1224.90 ms 1231.02 ms 6.12 ms

App size

Revision Plain With Sentry Diff
abb7058+dirty 2.92 MiB 3.43 MiB 524.53 KiB
e540498+dirty 2.92 MiB 3.69 MiB 794.14 KiB
d43a46b+dirty 2.92 MiB 3.64 MiB 740.29 KiB
0677344+dirty 2.92 MiB 3.41 MiB 500.94 KiB
148f924+dirty 2.92 MiB 3.60 MiB 701.88 KiB
700cbf4+dirty 2.92 MiB 3.64 MiB 740.57 KiB
22e31b6+dirty 2.92 MiB 3.43 MiB 524.74 KiB
5bb8d5f+dirty 2.92 MiB 3.48 MiB 575.85 KiB
e73d82f+dirty 2.92 MiB 3.64 MiB 740.56 KiB
70e6261+dirty 2.92 MiB 3.59 MiB 686.11 KiB

@github-actions GitHub Actions
Copy link
Contributor

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

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1217.96 ms 1220.41 ms 2.45 ms
Size 2.36 MiB 3.10 MiB 752.38 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
abb7058+dirty 1255.42 ms 1268.86 ms 13.44 ms
e540498+dirty 1220.61 ms 1212.93 ms -7.68 ms
d43a46b+dirty 1219.24 ms 1219.65 ms 0.41 ms
0677344+dirty 1276.70 ms 1300.07 ms 23.37 ms
148f924+dirty 1214.76 ms 1215.73 ms 0.97 ms
700cbf4+dirty 1234.59 ms 1227.71 ms -6.88 ms
22e31b6+dirty 1253.62 ms 1265.96 ms 12.34 ms
5bb8d5f+dirty 1235.47 ms 1237.39 ms 1.92 ms
e73d82f+dirty 1207.52 ms 1216.73 ms 9.21 ms
70e6261+dirty 1220.09 ms 1230.04 ms 9.95 ms

App size

Revision Plain With Sentry Diff
abb7058+dirty 2.36 MiB 2.87 MiB 520.42 KiB
e540498+dirty 2.36 MiB 3.14 MiB 793.34 KiB
d43a46b+dirty 2.36 MiB 3.08 MiB 734.25 KiB
0677344+dirty 2.36 MiB 2.85 MiB 496.81 KiB
148f924+dirty 2.36 MiB 3.04 MiB 696.25 KiB
700cbf4+dirty 2.36 MiB 3.08 MiB 734.22 KiB
22e31b6+dirty 2.36 MiB 2.87 MiB 520.67 KiB
5bb8d5f+dirty 2.36 MiB 2.92 MiB 570.22 KiB
e73d82f+dirty 2.36 MiB 3.08 MiB 734.23 KiB
70e6261+dirty 2.36 MiB 3.03 MiB 680.42 KiB

@krystofwoldrich
Copy link
Member Author

Looks like private static final Charset UTF_8 = Charset.forName(Charset.UTF_8); doesn't work.

@krystofwoldrich krystofwoldrich marked this pull request as draft October 16, 2024 11:01
@krystofwoldrich krystofwoldrich marked this pull request as ready for review October 16, 2024 11:29
@github-actions GitHub Actions
Copy link
Contributor

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 412.92 ms 451.04 ms 38.13 ms
Size 7.15 MiB 8.35 MiB 1.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5bb8d5f+dirty 356.71 ms 389.65 ms 32.94 ms
80b2ce3+dirty 271.29 ms 316.47 ms 45.18 ms
acadc0f+dirty 259.04 ms 304.67 ms 45.63 ms
dadc233+dirty 363.19 ms 370.37 ms 7.18 ms
0db0c72+dirty 335.20 ms 351.06 ms 15.86 ms
9433f35+dirty 265.50 ms 336.08 ms 70.58 ms
8ae23a7+dirty 398.10 ms 411.48 ms 13.38 ms
1d86dd6+dirty 335.76 ms 371.22 ms 35.46 ms
e73d82f+dirty 377.67 ms 407.06 ms 29.39 ms
baa882f+dirty 449.30 ms 540.40 ms 91.10 ms

App size

Revision Plain With Sentry Diff
5bb8d5f+dirty 7.15 MiB 8.21 MiB 1.06 MiB
80b2ce3+dirty 7.15 MiB 8.04 MiB 911.02 KiB
acadc0f+dirty 7.15 MiB 8.03 MiB 903.20 KiB
dadc233+dirty 7.15 MiB 8.04 MiB 910.84 KiB
0db0c72+dirty 7.15 MiB 8.04 MiB 911.02 KiB
9433f35+dirty 7.15 MiB 8.08 MiB 959.34 KiB
8ae23a7+dirty 7.15 MiB 8.35 MiB 1.20 MiB
1d86dd6+dirty 7.15 MiB 8.13 MiB 1002.18 KiB
e73d82f+dirty 7.15 MiB 8.34 MiB 1.19 MiB
baa882f+dirty 7.15 MiB 8.34 MiB 1.19 MiB

antonis
antonis approved these changes Oct 16, 2024
View reviewed changes
Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

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

The generated code changes LGTM and the script run as expected on CI and locally 🚀
Thank you for improving our tooling @krystofwoldrich 🙇

@krystofwoldrich krystofwoldrich merged commit fe13591 into main Oct 16, 2024
63 of 64 checks passed
@krystofwoldrich krystofwoldrich deleted the kw/add-pmd-static-analysis branch October 16, 2024 12:34
@github-actions GitHub Actions
Copy link
Contributor

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 479.54 ms 488.04 ms 8.50 ms
Size 17.74 MiB 20.07 MiB 2.34 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
70e6261 482.65 ms 495.70 ms 13.05 ms
457e29f 398.10 ms 421.39 ms 23.29 ms
3ffcddd 302.92 ms 315.80 ms 12.88 ms
484813b 434.55 ms 452.31 ms 17.75 ms
5571a20 410.55 ms 441.06 ms 30.51 ms
b95b8af 454.05 ms 454.53 ms 0.48 ms
e540498 436.26 ms 433.00 ms -3.26 ms
acadc0f+dirty 373.24 ms 381.51 ms 8.27 ms
dadc233+dirty 333.78 ms 343.94 ms 10.16 ms
1d86dd6 405.14 ms 411.06 ms 5.92 ms

App size

Revision Plain With Sentry Diff
70e6261 17.73 MiB 19.94 MiB 2.21 MiB
457e29f 17.73 MiB 19.84 MiB 2.10 MiB
3ffcddd 17.73 MiB 19.75 MiB 2.02 MiB
484813b 17.73 MiB 20.07 MiB 2.33 MiB
5571a20 17.73 MiB 19.93 MiB 2.19 MiB
b95b8af 17.73 MiB 20.11 MiB 2.37 MiB
e540498 17.73 MiB 20.11 MiB 2.37 MiB
acadc0f+dirty 17.73 MiB 19.75 MiB 2.01 MiB
dadc233+dirty 17.73 MiB 19.75 MiB 2.02 MiB
1d86dd6 17.73 MiB 19.86 MiB 2.12 MiB

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

@antonis antonis antonis approved these changes

@lucas-zimerman lucas-zimerman Awaiting requested review from lucas-zimerman lucas-zimerman 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.
2 participants
@krystofwoldrich @antonis