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

fix: profiling app release version format #2470

Merged
merged 5 commits into from
Dec 1, 2022

Conversation

armcknight
Copy link
Member

While pairing, @Zylphrex noticed we gather an app's version info into a format that isn't compatible with other parts of our backend, and this could break connections in unexpected ways. This PR changes it to use the same format, which was already composed by another area in the SDK.

@armcknight armcknight changed the base branch from master to 8.0.0 December 1, 2022 03:47
@github-actions
Copy link

github-actions bot commented Dec 1, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1251.94 ms 1275.78 ms 23.84 ms
Size 20.75 KiB 383.77 KiB 363.02 KiB

Baseline results on branch: 8.0.0

Startup times

Revision Plain With Sentry Diff
42bb369 1218.49 ms 1241.98 ms 23.49 ms
a9e77dc 1231.94 ms 1254.85 ms 22.91 ms
d446105 1237.06 ms 1261.34 ms 24.28 ms
a0c8696 1239.33 ms 1261.92 ms 22.59 ms
dd266f5 1224.76 ms 1252.63 ms 27.87 ms
f515911 1236.75 ms 1261.56 ms 24.81 ms
3936dd2 1214.98 ms 1242.24 ms 27.26 ms
dcac8ad 1238.82 ms 1247.80 ms 8.98 ms
ffa889e 1229.37 ms 1253.38 ms 24.01 ms
58ec104 1244.29 ms 1269.67 ms 25.39 ms

App size

Revision Plain With Sentry Diff
42bb369 20.75 KiB 383.87 KiB 363.12 KiB
a9e77dc 20.75 KiB 379.12 KiB 358.36 KiB
d446105 20.75 KiB 383.37 KiB 362.62 KiB
a0c8696 20.75 KiB 383.89 KiB 363.14 KiB
dd266f5 20.75 KiB 383.39 KiB 362.64 KiB
f515911 20.75 KiB 383.87 KiB 363.12 KiB
3936dd2 20.75 KiB 383.29 KiB 362.53 KiB
dcac8ad 20.75 KiB 379.11 KiB 358.36 KiB
ffa889e 20.75 KiB 383.83 KiB 363.08 KiB
58ec104 20.75 KiB 379.11 KiB 358.36 KiB

Previous results on branch: armcknight/fix/release-format

Startup times

Revision Plain With Sentry Diff
954f893 1235.18 ms 1276.00 ms 40.82 ms
dcb0bf5 1229.72 ms 1249.72 ms 20.00 ms
cc74ef2 1203.63 ms 1237.62 ms 33.99 ms

App size

Revision Plain With Sentry Diff
954f893 20.75 KiB 383.77 KiB 363.02 KiB
dcb0bf5 20.75 KiB 383.38 KiB 362.63 KiB
cc74ef2 20.75 KiB 383.39 KiB 362.63 KiB

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

LGTM

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.

LGTM

@philipphofmann
Copy link
Member

philipphofmann commented Dec 1, 2022

I merged 8.0.0 into this branch cause I think it has a fix for the failing lint check. So I save you some time, @armcknight.

@codecov
Copy link

codecov bot commented Dec 1, 2022

Codecov Report

❗ No coverage uploaded for pull request base (8.0.0@27443e0). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             8.0.0    #2470   +/-   ##
========================================
  Coverage         ?   88.42%           
========================================
  Files            ?      178           
  Lines            ?    15204           
  Branches         ?     5280           
========================================
  Hits             ?    13444           
  Misses           ?     1582           
  Partials         ?      178           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27443e0...de2676e. Read the comment docs.

@armcknight armcknight merged commit d8c347f into 8.0.0 Dec 1, 2022
@armcknight armcknight deleted the armcknight/fix/release-format branch December 1, 2022 22:05
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.

3 participants