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

ref: Make SentrySession and SentrySDKInfo internal #2451

Merged
merged 5 commits into from
Dec 1, 2022

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Nov 28, 2022

📜 Description

There is no reason why SentrySession and SentrySDKInfo should be public. We can make it internal.

💡 Motivation and Context

Came up in #2447 (comment)

💚 How did you test it?

CI

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

There is no reason why SentrySession should be public.
We can make it internal.
@philipphofmann philipphofmann changed the title ref: Make SentrySession internal ref: Make SentrySession and SentrySDKInfo internal Nov 28, 2022
@github-actions
Copy link

github-actions bot commented Nov 28, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1229.56 ms 1252.68 ms 23.12 ms
Size 20.75 KiB 383.38 KiB 362.62 KiB

Baseline results on branch: 8.0.0

Startup times

Revision Plain With Sentry Diff
d10145a 1232.65 ms 1257.55 ms 24.90 ms
909e73a 1217.78 ms 1229.70 ms 11.92 ms
1ce879f 1258.12 ms 1260.90 ms 2.78 ms
3fdb749 1227.42 ms 1248.48 ms 21.06 ms
9f8d429 1239.57 ms 1255.12 ms 15.55 ms
9cb6c52 1201.08 ms 1211.37 ms 10.29 ms
e2cec76 1189.48 ms 1229.84 ms 40.36 ms
fb9f27a 1218.55 ms 1249.17 ms 30.62 ms
c9129b6 1231.86 ms 1270.11 ms 38.25 ms
dcac8ad 1238.82 ms 1247.80 ms 8.98 ms

App size

Revision Plain With Sentry Diff
d10145a 20.75 KiB 379.12 KiB 358.36 KiB
909e73a 20.75 KiB 383.40 KiB 362.65 KiB
1ce879f 20.75 KiB 381.81 KiB 361.06 KiB
3fdb749 20.75 KiB 383.81 KiB 363.06 KiB
9f8d429 20.75 KiB 383.40 KiB 362.65 KiB
9cb6c52 20.75 KiB 382.12 KiB 361.36 KiB
e2cec76 20.75 KiB 381.81 KiB 361.06 KiB
fb9f27a 20.75 KiB 382.11 KiB 361.36 KiB
c9129b6 20.75 KiB 381.81 KiB 361.06 KiB
dcac8ad 20.75 KiB 379.11 KiB 358.36 KiB

Previous results on branch: ref/session-internal

Startup times

Revision Plain With Sentry Diff
c7ef8f8 1220.18 ms 1246.88 ms 26.70 ms
8f5c39e 1253.94 ms 1271.53 ms 17.59 ms
9315308 1244.20 ms 1252.60 ms 8.40 ms

App size

Revision Plain With Sentry Diff
c7ef8f8 20.75 KiB 383.38 KiB 362.62 KiB
8f5c39e 20.75 KiB 383.78 KiB 363.02 KiB
9315308 20.75 KiB 383.78 KiB 363.03 KiB

Copy link
Contributor

@kevinrenskers kevinrenskers left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann philipphofmann merged commit f515911 into 8.0.0 Dec 1, 2022
@philipphofmann philipphofmann deleted the ref/session-internal branch December 1, 2022 09:16
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.

2 participants