fix: Fix FileManager logs warning for .DS_Store#3584
Conversation
Don't log a warning in SentryFileManager for .DS_Store files.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c471221 | 1244.57 ms | 1257.94 ms | 13.37 ms |
| ea2a263 | 1201.84 ms | 1214.78 ms | 12.94 ms |
| f25febb | 1224.69 ms | 1247.38 ms | 22.68 ms |
| 7e8d5fd | 6249.23 ms | 6253.90 ms | 4.67 ms |
| efb2222 | 1245.10 ms | 1249.10 ms | 4.00 ms |
| 881a955 | 1214.27 ms | 1235.88 ms | 21.61 ms |
| b1b7d72 | 1246.31 ms | 1261.00 ms | 14.69 ms |
| 8636ef0 | 1239.96 ms | 1253.14 ms | 13.18 ms |
| 9e6665f | 1233.33 ms | 1257.14 ms | 23.81 ms |
| a5c946b | 1219.33 ms | 1236.53 ms | 17.20 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c471221 | 22.85 KiB | 413.89 KiB | 391.04 KiB |
| ea2a263 | 21.58 KiB | 418.58 KiB | 396.99 KiB |
| f25febb | 21.58 KiB | 414.92 KiB | 393.34 KiB |
| 7e8d5fd | 20.76 KiB | 435.50 KiB | 414.74 KiB |
| efb2222 | 20.76 KiB | 424.45 KiB | 403.69 KiB |
| 881a955 | 22.85 KiB | 407.63 KiB | 384.78 KiB |
| b1b7d72 | 20.76 KiB | 436.33 KiB | 415.57 KiB |
| 8636ef0 | 22.84 KiB | 403.18 KiB | 380.34 KiB |
| 9e6665f | 22.84 KiB | 403.52 KiB | 380.67 KiB |
| a5c946b | 20.76 KiB | 431.93 KiB | 411.17 KiB |
Previous results on branch: fix/filemanager-log-message-dsstore
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 74c46b1 | 1221.11 ms | 1251.33 ms | 30.21 ms |
| 354a3a2 | 1230.19 ms | 1246.80 ms | 16.62 ms |
| 6b3f991 | 1210.87 ms | 1221.43 ms | 10.55 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 74c46b1 | 21.58 KiB | 418.20 KiB | 396.62 KiB |
| 354a3a2 | 21.58 KiB | 418.19 KiB | 396.61 KiB |
| 6b3f991 | 21.58 KiB | 418.07 KiB | 396.49 KiB |
|
Still need to fix the tests, sorry. |
armcknight
left a comment
armcknight
left a comment
There was a problem hiding this comment.
I'll wait to review until the tests pass, but there was one small detail I figured I'd comment on.
| - (void)deleteOldEnvelopeItems; | ||
|
|
||
| /** | ||
| * Only used for testing. |
There was a problem hiding this comment.
Could we create a SentryFileManager+Test.h in the test target to expose this?
There was a problem hiding this comment.
The FileManager isn't public, so I often don't add the extra header file. It's just extra work. Still, if you'd like, we could add one.
There was a problem hiding this comment.
@armcknight, I'm going to merge this PR and maybe open another PR, depending on your reply.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3584 +/- ##
=============================================
+ Coverage 89.235% 89.282% +0.047%
=============================================
Files 529 529
Lines 57588 57869 +281
Branches 20364 20709 +345
=============================================
+ Hits 51389 51667 +278
- Misses 5284 5291 +7
+ Partials 915 911 -4
... and 72 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
3 participants
📜 Description
Don't log a warning in SentryFileManager for .DS_Store files.
💡 Motivation and Context
Fixes one part of GH-3577
💚 How did you test it?
Unit tests
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.🔮 Next steps