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.

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

chore: Organize sample #2928

Merged
merged 17 commits into from
Apr 26, 2023
Merged

chore: Organize sample #2928

merged 17 commits into from
Apr 26, 2023

Conversation

brustolin
Copy link
Contributor

@brustolin brustolin commented Apr 18, 2023

Organize iOS-Swift sample by dividing diverent types of operations in different tabs given more room for more tests.

The result:
sample

#skip-changelog

@github-actions
Copy link

github-actions bot commented Apr 18, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1250.71 ms 1267.80 ms 17.09 ms
Size 20.76 KiB 434.65 KiB 413.89 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
c9724f9 1199.38 ms 1229.54 ms 30.16 ms
66922ca 1221.68 ms 1235.98 ms 14.30 ms
869ab7e 1230.36 ms 1255.49 ms 25.13 ms
a9103fe 1265.47 ms 1267.37 ms 1.90 ms
ecd9ecd 1204.67 ms 1226.46 ms 21.79 ms
4977fbc 1231.55 ms 1239.80 ms 8.25 ms
4b08ceb 1237.75 ms 1249.61 ms 11.86 ms
7f691b5 1233.94 ms 1243.80 ms 9.86 ms
a6f8b18 1236.96 ms 1240.14 ms 3.18 ms
b6ba04e 1217.45 ms 1248.92 ms 31.47 ms

App size

Revision Plain With Sentry Diff
c9724f9 20.76 KiB 427.66 KiB 406.90 KiB
66922ca 20.76 KiB 425.80 KiB 405.04 KiB
869ab7e 20.76 KiB 432.88 KiB 412.11 KiB
a9103fe 20.76 KiB 426.95 KiB 406.19 KiB
ecd9ecd 20.76 KiB 420.23 KiB 399.47 KiB
4977fbc 20.76 KiB 419.86 KiB 399.10 KiB
4b08ceb 20.76 KiB 431.99 KiB 411.23 KiB
7f691b5 20.76 KiB 420.55 KiB 399.79 KiB
a6f8b18 20.76 KiB 431.87 KiB 411.11 KiB
b6ba04e 20.76 KiB 414.45 KiB 393.69 KiB

Previous results on branch: chore/update-sample

Startup times

Revision Plain With Sentry Diff
53a4f2a 1233.88 ms 1260.04 ms 26.16 ms
12f8de5 1222.90 ms 1247.38 ms 24.49 ms
2c8ced6 1237.12 ms 1246.35 ms 9.22 ms
9b10786 1223.67 ms 1245.28 ms 21.61 ms
87b5a8c 1230.14 ms 1260.06 ms 29.92 ms
533ec22 1209.51 ms 1228.08 ms 18.57 ms
75d78ed 1195.61 ms 1214.04 ms 18.43 ms

App size

Revision Plain With Sentry Diff
53a4f2a 20.76 KiB 433.18 KiB 412.42 KiB
12f8de5 20.76 KiB 432.87 KiB 412.11 KiB
2c8ced6 20.76 KiB 434.92 KiB 414.16 KiB
9b10786 20.76 KiB 432.88 KiB 412.11 KiB
87b5a8c 20.76 KiB 433.18 KiB 412.42 KiB
533ec22 20.76 KiB 434.92 KiB 414.16 KiB
75d78ed 20.76 KiB 432.88 KiB 412.12 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Can we please make the buttons a bit bigger at the bottom?

Screenshot 2023-04-21 at 12 05 02

It would be great if it looked a bit more navigation like.

@marandaneto
Copy link
Contributor

marandaneto commented Apr 21, 2023

@brustolin can we improve the labels to be a bit more clear of the outcome when clicking on the buttons?

  • What is NiB?
  • What should we see on Sentry when clicking on TableView, SplitView, CoreData, Performance scenarios, etc...

Maybe we could always add the intention as a prefix/suffix.
For example, CoreData spans, Table view (add intention), etc.
I'd need to read the source code to understand what they are doing as of now.

@brustolin
Copy link
Contributor Author

@marandaneto I think we can improve this a little bit.

Most of this actions don't really matter for a booth sample, we use to open the various types of view controllers to make sure our code don't break anything, but most of the view controllers actions will just create a transaction.

@brustolin
Copy link
Contributor Author

Can we please make the buttons a bit bigger at the bottom?

We can choose an Icon to go along, this is the default tab item font size

@brustolin brustolin marked this pull request as ready for review April 21, 2023 13:14
@brustolin brustolin requested a review from armcknight as a code owner April 21, 2023 13:14
}

override func tearDown() {
app.terminate()
super.tearDown()
}

func testCrashRecovery() {
Copy link
Member

Choose a reason for hiding this comment

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

is this a typo?

Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

LGTM, I think we should forego any other cosmetic concerns, which can be iterated on in subsequent PRs. I still see this more as a test harness than demo app so am personally not concerned with that kind of thing.

@brustolin
Copy link
Contributor Author

Updated version of the button labels and tabs

sample

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Thanks @brustolin 👏

@brustolin brustolin merged commit 53a8885 into main Apr 26, 2023
@brustolin brustolin deleted the chore/update-sample branch April 26, 2023 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants