- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 371
fix: Remove unused SentryFrame.instruction #6504
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
Conversation
The property isn't used anywhere in the SDK, we can remove it. Fixes GH-4738
| Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@              Coverage Diff              @@
##              main     #6504       +/-   ##
=============================================
+ Coverage   85.458%   85.955%   +0.497%     
=============================================
  Files          451       451               
  Lines        27328     27328               
  Branches     11917     11918        +1     
=============================================
+ Hits         23354     23490      +136     
+ Misses        3929      3793      -136     
  Partials        45        45               see 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry. 
 | 
| Performance metrics 🚀
 
 | 
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| 9264ee8 | 1233.15 ms | 1250.02 ms | 16.87 ms | 
| 07d7e83 | 1211.71 ms | 1240.08 ms | 28.37 ms | 
| 119ab1c | 1226.79 ms | 1254.55 ms | 27.76 ms | 
| 66147d5 | 1234.45 ms | 1268.45 ms | 34.00 ms | 
| fa9a4bb | 1217.91 ms | 1248.72 ms | 30.81 ms | 
| be882e4 | 1199.35 ms | 1231.20 ms | 31.86 ms | 
| d7461dc | 1233.69 ms | 1255.29 ms | 21.60 ms | 
| f97a070 | 1218.88 ms | 1253.12 ms | 34.24 ms | 
| 339539a | 1219.58 ms | 1254.63 ms | 35.05 ms | 
| 26f7b17 | 1218.47 ms | 1253.82 ms | 35.35 ms | 
App size
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| 9264ee8 | 23.75 KiB | 913.09 KiB | 889.34 KiB | 
| 07d7e83 | 23.75 KiB | 913.27 KiB | 889.52 KiB | 
| 119ab1c | 23.75 KiB | 993.70 KiB | 969.95 KiB | 
| 66147d5 | 23.74 KiB | 1.01 MiB | 1008.77 KiB | 
| fa9a4bb | 23.75 KiB | 986.91 KiB | 963.16 KiB | 
| be882e4 | 23.75 KiB | 946.69 KiB | 922.94 KiB | 
| d7461dc | 23.75 KiB | 874.45 KiB | 850.70 KiB | 
| f97a070 | 23.75 KiB | 858.68 KiB | 834.93 KiB | 
| 339539a | 23.75 KiB | 968.24 KiB | 944.50 KiB | 
| 26f7b17 | 23.75 KiB | 960.93 KiB | 937.19 KiB | 
Previous results on branch: fix/remove-frame-instruction
Startup times
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| d075be2 | 1214.22 ms | 1240.77 ms | 26.54 ms | 
| 2092242 | 1207.98 ms | 1253.48 ms | 45.50 ms | 
| 97add6b | 1231.61 ms | 1269.67 ms | 38.06 ms | 
| 150ed54 | 1221.74 ms | 1254.50 ms | 32.76 ms | 
App size
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| d075be2 | 23.75 KiB | 1.00 MiB | 1002.29 KiB | 
| 2092242 | 23.75 KiB | 1.01 MiB | 1006.38 KiB | 
| 97add6b | 23.75 KiB | 1.01 MiB | 1006.40 KiB | 
| 150ed54 | 23.75 KiB | 1.00 MiB | 1005.11 KiB | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, maybe worth noting here that this is a breaking change but we are still preparing for v9
| Yes, it's a breaking change. That's why I also added it to the breaking changes in the changelog. | 
📜 Description
The property isn't used anywhere in the SDK, we can remove it.
💡 Motivation and Context
Fixes GH-4738
💚 How did you test it?
CI is still green.
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.