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: replace magic number with equivalent constant #4123

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

armcknight
Copy link
Member

I noticed this hanging around at some point.

#skip-changelog

Copy link

codecov bot commented Jun 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.355%. Comparing base (df50619) to head (380a609).
Report is 426 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4123       +/-   ##
=============================================
+ Coverage   91.338%   91.355%   +0.016%     
=============================================
  Files          610       609        -1     
  Lines        48421     48377       -44     
  Branches     17351     17344        -7     
=============================================
- Hits         44227     44195       -32     
+ Misses        4101      4091       -10     
+ Partials        93        91        -2     
Files with missing lines Coverage Δ
Sources/Sentry/SentryOptions.m 99.244% <100.000%> (ø)

... and 27 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

Copy link

github-actions bot commented Jun 26, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.08 ms 1252.71 ms 18.63 ms
Size 21.58 KiB 678.19 KiB 656.61 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
853d797 1221.40 ms 1233.98 ms 12.58 ms
c0ff306 1219.24 ms 1243.96 ms 24.72 ms
c838244 1213.51 ms 1243.76 ms 30.24 ms
2ccbd03 1225.13 ms 1247.51 ms 22.39 ms
699d76f 1233.96 ms 1251.32 ms 17.36 ms
4386045 1225.67 ms 1244.90 ms 19.23 ms
48e8c2e 1226.45 ms 1246.85 ms 20.41 ms
c8dbe73 1215.37 ms 1237.31 ms 21.94 ms
326b7eb 1204.76 ms 1234.96 ms 30.20 ms
407ff99 1216.63 ms 1235.50 ms 18.87 ms

App size

Revision Plain With Sentry Diff
853d797 21.58 KiB 630.35 KiB 608.77 KiB
c0ff306 20.76 KiB 434.65 KiB 413.89 KiB
c838244 21.58 KiB 629.61 KiB 608.03 KiB
2ccbd03 21.58 KiB 546.20 KiB 524.62 KiB
699d76f 21.58 KiB 631.82 KiB 610.24 KiB
4386045 22.84 KiB 403.07 KiB 380.23 KiB
48e8c2e 21.58 KiB 418.44 KiB 396.86 KiB
c8dbe73 21.58 KiB 615.91 KiB 594.33 KiB
326b7eb 20.76 KiB 432.31 KiB 411.55 KiB
407ff99 20.76 KiB 427.87 KiB 407.10 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.

Just one question.
The const is not being used, but should we use it?

@armcknight
Copy link
Member Author

Just one question. The const is not being used, but should we use it?

So looking at the git blame for the definition of the constant:

static NSTimeInterval const SentryTracerDefaultTimeout = 3.0;

and the hardcoded value of this option's default:

self.idleTimeout = 3.0;

they both came from #1784. I guess it was just forgotten to actually assign the constant, so this PR now does that.

@armcknight armcknight merged commit efd8ddb into main Jun 28, 2024
66 checks passed
@armcknight armcknight deleted the armcknight/ref/remove-unused-constant branch June 28, 2024 22:16
@armcknight armcknight changed the title ref: remove unused constant ref: replace magic number with equivalent constant Feb 1, 2025
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