chore(sample): Remove old sample in favour of the new arch sample #3118

krystofwoldrich · 2023-06-13T13:14:11Z

📢 Type of change

Refactoring

📜 Description

The original sample is not being updated and the update rn ci script stopped working. Instead of trying to fix it, I've removed the old sample and fixed ci to use the new sample in places where it was needed.

The update-rn.sh originally updated the rn version in the sample. I've changed it to update the dev dep RN version of the SDK, so our SDK is always built with the latest RN.

update-rn.sh will be updated in the future to actually update the RN version in all the places thru the repo. But for now, let's limit it only to the SDK package.json.

💚 How did you test it?

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

#skip-changelog

github-actions · 2023-06-13T13:35:35Z

Android (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	329.47 ms	340.39 ms	10.92 ms
Size	17.73 MiB	19.80 MiB	2.07 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`b1e8712`	462.11 ms	465.71 ms	3.60 ms
`acadc0f`+dirty	373.24 ms	381.51 ms	8.27 ms
`ad6c299`	375.94 ms	382.02 ms	6.08 ms
`34aba08`	328.10 ms	342.84 ms	14.74 ms
`80b2ce3`	385.02 ms	387.36 ms	2.34 ms
`d7401ac`+dirty	375.20 ms	383.51 ms	8.31 ms
`e2b64fe`	316.88 ms	330.23 ms	13.35 ms
`0db0c72`	372.12 ms	386.00 ms	13.88 ms
`76d1baf`+dirty	335.72 ms	355.52 ms	19.80 ms
`dadc233`+dirty	333.78 ms	343.94 ms	10.16 ms

App size

Revision	Plain	With Sentry	Diff
`b1e8712`	17.73 MiB	19.75 MiB	2.02 MiB
`acadc0f`+dirty	17.73 MiB	19.75 MiB	2.01 MiB
`ad6c299`	17.73 MiB	19.75 MiB	2.02 MiB
`34aba08`	17.73 MiB	19.80 MiB	2.07 MiB
`80b2ce3`	17.73 MiB	19.75 MiB	2.02 MiB
`d7401ac`+dirty	17.73 MiB	19.75 MiB	2.02 MiB
`e2b64fe`	17.73 MiB	19.80 MiB	2.07 MiB
`0db0c72`	17.73 MiB	19.75 MiB	2.02 MiB
`76d1baf`+dirty	17.73 MiB	20.04 MiB	2.31 MiB
`dadc233`+dirty	17.73 MiB	19.75 MiB	2.02 MiB

github-actions · 2023-06-13T13:45:54Z

iOS (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1284.55 ms	1317.04 ms	32.49 ms
Size	2.36 MiB	2.85 MiB	495.76 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`3ffcddd`+dirty	1244.47 ms	1264.14 ms	19.67 ms
`acadc0f`+dirty	1264.38 ms	1290.06 ms	25.68 ms
`d7401ac`+dirty	1252.38 ms	1275.04 ms	22.66 ms
`76d1baf`+dirty	1244.10 ms	1268.52 ms	24.42 ms
`dadc233`+dirty	1223.20 ms	1236.88 ms	13.68 ms
`80b2ce3`+dirty	1265.92 ms	1268.60 ms	2.69 ms
`70caa60`+dirty	1218.27 ms	1230.30 ms	12.03 ms
`52a8031`+dirty	1280.88 ms	1289.78 ms	8.90 ms
`d197b5c`+dirty	1217.61 ms	1242.66 ms	25.05 ms
`15c80ab`+dirty	1223.74 ms	1228.96 ms	5.22 ms

App size

Revision	Plain	With Sentry	Diff
`3ffcddd`+dirty	2.36 MiB	2.84 MiB	489.60 KiB
`acadc0f`+dirty	2.36 MiB	2.83 MiB	480.37 KiB
`d7401ac`+dirty	2.36 MiB	2.83 MiB	481.14 KiB
`76d1baf`+dirty	2.36 MiB	2.82 MiB	469.45 KiB
`dadc233`+dirty	2.36 MiB	2.84 MiB	486.85 KiB
`80b2ce3`+dirty	2.36 MiB	2.84 MiB	486.98 KiB
`70caa60`+dirty	2.36 MiB	2.83 MiB	479.27 KiB
`52a8031`+dirty	2.36 MiB	2.82 MiB	469.44 KiB
`d197b5c`+dirty	2.36 MiB	2.82 MiB	462.86 KiB
`15c80ab`+dirty	2.36 MiB	2.83 MiB	474.49 KiB

package.json

.github/workflows/sample-application.yml

lucas-zimerman · 2023-06-19T13:25:30Z

Nice one!

A few points:

I see you removed the old sample but it seems like some references about it weren't removed, some of them I can't request a change since it's none of the files were altered, but here are the files:

.eslintignore
https://github.com/getsentry/sentry-react-native/blob/main/.eslintignore#L5

.vscode
https://github.com/getsentry/sentry-react-native/blob/main/.vscode/settings.json#L26

Other than those minor changes, it looks ready for merge, thank you!

krystofwoldrich · 2023-06-19T19:14:49Z

@lucas-zimerman Thank you, I've missed those references.

github-actions · 2023-06-19T20:05:44Z

iOS (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	6239.53 ms	6283.92 ms	44.39 ms
Size	2.92 MiB	3.41 MiB	499.92 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`3ffcddd`+dirty	1272.22 ms	1273.98 ms	1.76 ms
`acadc0f`+dirty	1271.12 ms	1272.28 ms	1.16 ms
`d7401ac`+dirty	1288.10 ms	1289.54 ms	1.44 ms
`76d1baf`+dirty	1245.00 ms	1257.76 ms	12.76 ms
`dadc233`+dirty	1266.52 ms	1282.55 ms	16.03 ms
`80b2ce3`+dirty	1245.12 ms	1262.04 ms	16.92 ms
`70caa60`+dirty	1279.08 ms	1281.54 ms	2.46 ms
`52a8031`+dirty	1255.96 ms	1273.00 ms	17.04 ms
`d197b5c`+dirty	1234.80 ms	1249.20 ms	14.40 ms
`15c80ab`+dirty	1248.41 ms	1251.24 ms	2.83 ms

App size

Revision	Plain	With Sentry	Diff
`3ffcddd`+dirty	2.92 MiB	3.40 MiB	494.39 KiB
`acadc0f`+dirty	2.92 MiB	3.39 MiB	487.34 KiB
`d7401ac`+dirty	2.92 MiB	3.40 MiB	488.06 KiB
`76d1baf`+dirty	2.92 MiB	3.38 MiB	475.74 KiB
`dadc233`+dirty	2.92 MiB	3.40 MiB	492.53 KiB
`80b2ce3`+dirty	2.92 MiB	3.40 MiB	492.75 KiB
`70caa60`+dirty	2.92 MiB	3.39 MiB	486.04 KiB
`52a8031`+dirty	2.92 MiB	3.38 MiB	475.71 KiB
`d197b5c`+dirty	2.92 MiB	3.37 MiB	464.41 KiB
`15c80ab`+dirty	2.92 MiB	3.39 MiB	481.56 KiB

github-actions · 2023-06-19T20:13:08Z

Android (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	269.71 ms	300.57 ms	30.87 ms
Size	7.15 MiB	8.07 MiB	947.16 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`3ffcddd`+dirty	244.82 ms	295.47 ms	50.65 ms
`acadc0f`+dirty	259.04 ms	304.67 ms	45.63 ms
`d7401ac`+dirty	373.98 ms	394.08 ms	20.10 ms
`76d1baf`+dirty	339.02 ms	408.65 ms	69.63 ms
`dadc233`+dirty	363.19 ms	370.37 ms	7.18 ms
`80b2ce3`+dirty	271.29 ms	316.47 ms	45.18 ms
`70caa60`+dirty	308.83 ms	393.06 ms	84.23 ms
`52a8031`+dirty	330.72 ms	358.76 ms	28.03 ms
`d197b5c`+dirty	258.75 ms	313.61 ms	54.86 ms
`15c80ab`+dirty	276.38 ms	327.54 ms	51.17 ms

App size

Revision	Plain	With Sentry	Diff
`3ffcddd`+dirty	7.15 MiB	8.04 MiB	912.17 KiB
`acadc0f`+dirty	7.15 MiB	8.03 MiB	903.20 KiB
`d7401ac`+dirty	7.15 MiB	8.04 MiB	910.85 KiB
`76d1baf`+dirty	7.15 MiB	8.09 MiB	964.41 KiB
`dadc233`+dirty	7.15 MiB	8.04 MiB	910.84 KiB
`80b2ce3`+dirty	7.15 MiB	8.04 MiB	911.02 KiB
`70caa60`+dirty	7.15 MiB	8.03 MiB	901.79 KiB
`52a8031`+dirty	7.15 MiB	8.09 MiB	965.95 KiB
`d197b5c`+dirty	7.15 MiB	8.09 MiB	962.72 KiB
`15c80ab`+dirty	7.15 MiB	8.09 MiB	966.13 KiB

marandaneto · 2023-06-20T06:24:22Z

@krystofwoldrich if we don't build the old sample, how do we know that the SDK is compatible with older versions and new architecture disabled?

krystofwoldrich · 2023-06-20T12:28:01Z

@marandaneto We have separate ci jobs that create plain RN app of given RN version, add Sentry, build the app with given arch (new/legacy) and run e2e on that app.

https://github.com/getsentry/sentry-react-native/actions/runs/5315403231/jobs/9623722682?pr=3118

sentry-react-native/.github/workflows/e2e.yml

Line 244 in e727df8

react-native-build:

The new sample is built both for legacy and new arch.
https://github.com/getsentry/sentry-react-native/actions/runs/5315403229/jobs/9623722294?pr=3118

krystofwoldrich added 2 commits June 13, 2023 15:08

chore(sample): Remove old sample in favour of the new arch sample

aaba8c2

Add new update rn script updating sdks dev version of rn

4b4b8f3

Fix ci build pwd

0b40c1b

krystofwoldrich marked this pull request as ready for review June 13, 2023 17:21

krystofwoldrich requested review from marandaneto and lucas-zimerman as code owners June 13, 2023 17:21

lucas-zimerman reviewed Jun 19, 2023

View reviewed changes

package.json Show resolved Hide resolved

lucas-zimerman reviewed Jun 19, 2023

View reviewed changes

.github/workflows/sample-application.yml Show resolved Hide resolved

lucas-zimerman approved these changes Jun 19, 2023

View reviewed changes

Remove last remaining references to the old sample

fd1b41a

krystofwoldrich merged commit 2d3b9f2 into main Jun 20, 2023
49 checks passed

krystofwoldrich deleted the kw-remove-old-sample branch June 20, 2023 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(sample): Remove old sample in favour of the new arch sample #3118

chore(sample): Remove old sample in favour of the new arch sample #3118

krystofwoldrich commented Jun 13, 2023

github-actions bot commented Jun 13, 2023 •

edited

Loading

Baseline results on branch: main

Startup times

App size

github-actions bot commented Jun 13, 2023 •

edited

Loading

Baseline results on branch: main

Startup times

App size

lucas-zimerman commented Jun 19, 2023

krystofwoldrich commented Jun 19, 2023

github-actions bot commented Jun 19, 2023

Baseline results on branch: main

Startup times

App size

github-actions bot commented Jun 19, 2023

Baseline results on branch: main

Startup times

App size

marandaneto commented Jun 20, 2023

krystofwoldrich commented Jun 20, 2023

chore(sample): Remove old sample in favour of the new arch sample #3118

chore(sample): Remove old sample in favour of the new arch sample #3118

Conversation

krystofwoldrich commented Jun 13, 2023

📢 Type of change

📜 Description

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Jun 13, 2023 • edited Loading

Android (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

github-actions bot commented Jun 13, 2023 • edited Loading

iOS (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

lucas-zimerman commented Jun 19, 2023

krystofwoldrich commented Jun 19, 2023

github-actions bot commented Jun 19, 2023

iOS (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

github-actions bot commented Jun 19, 2023

Android (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

marandaneto commented Jun 20, 2023

krystofwoldrich commented Jun 20, 2023

github-actions bot commented Jun 13, 2023 •

edited

Loading

github-actions bot commented Jun 13, 2023 •

edited

Loading