-
-
Notifications
You must be signed in to change notification settings - Fork 371
fix: Development builds with prebuilt binary #6989
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
base: main
Are you sure you want to change the base?
Conversation
| options: [], | ||
| format: &format), | ||
| let dict = obj as? [String: Any] else { | ||
| return |
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.
Bug: Tests use an old, non-existent property name, causing compilation errors.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The property mobileProvisionProfileProvisionsAllDevices was renamed to provisionsAllDevices in SentryMobileProvisionParser. While the Objective-C code was updated, 11 references in SentryMobileProvisionParserTests.swift still use the old property name. This will cause compilation errors in the test file, preventing the project from building successfully.
💡 Suggested Fix
Update all 11 occurrences of parser.mobileProvisionProfileProvisionsAllDevices in SentryMobileProvisionParserTests.swift to parser.provisionsAllDevices.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: Sources/Swift/Helper/SentryMobileProvisionParser.swift#L57
Potential issue: The property `mobileProvisionProfileProvisionsAllDevices` was renamed
to `provisionsAllDevices` in `SentryMobileProvisionParser`. While the Objective-C code
was updated, 11 references in `SentryMobileProvisionParserTests.swift` still use the old
property name. This will cause compilation errors in the test file, preventing the
project from building successfully.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5544207
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6989 +/- ##
=============================================
- Coverage 85.076% 7.064% -78.013%
=============================================
Files 453 418 -35
Lines 27674 26415 -1259
Branches 12166 9921 -2245
=============================================
- Hits 23544 1866 -21678
- Misses 4087 24536 +20449
+ Partials 43 13 -30
... and 434 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
itaybre
left a comment
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.
Looks good, but can you add some tests to ensure future changes don't break anything?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6989 +/- ##
=============================================
- Coverage 85.076% 7.064% -78.013%
=============================================
Files 453 418 -35
Lines 27674 26415 -1259
Branches 12166 9921 -2245
=============================================
- Hits 23544 1866 -21678
- Misses 4087 24536 +20449
+ Partials 43 13 -30
... and 434 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 37900c9 | 1236.06 ms | 1262.22 ms | 26.16 ms |
| 929c622 | 1228.48 ms | 1253.55 ms | 25.07 ms |
| 48e5c8a | 1223.02 ms | 1242.38 ms | 19.35 ms |
| a598105 | 1236.51 ms | 1258.88 ms | 22.37 ms |
| 319fb1e | 1219.48 ms | 1242.69 ms | 23.21 ms |
| e98d6f5 | 1228.51 ms | 1258.85 ms | 30.34 ms |
| daeb716 | 1215.41 ms | 1246.52 ms | 31.11 ms |
| cd67f52 | 1216.29 ms | 1255.27 ms | 38.99 ms |
| e8e40fd | 1222.10 ms | 1259.91 ms | 37.80 ms |
| 78af7a9 | 1225.75 ms | 1256.98 ms | 31.23 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 37900c9 | 23.75 KiB | 958.61 KiB | 934.85 KiB |
| 929c622 | 24.14 KiB | 1.02 MiB | 1016.90 KiB |
| 48e5c8a | 23.75 KiB | 913.63 KiB | 889.88 KiB |
| a598105 | 23.75 KiB | 968.24 KiB | 944.49 KiB |
| 319fb1e | 23.75 KiB | 1019.18 KiB | 995.43 KiB |
| e98d6f5 | 23.75 KiB | 933.03 KiB | 909.28 KiB |
| daeb716 | 23.75 KiB | 928.16 KiB | 904.41 KiB |
| cd67f52 | 23.75 KiB | 1.01 MiB | 1016.04 KiB |
| e8e40fd | 23.75 KiB | 1022.79 KiB | 999.04 KiB |
| 78af7a9 | 23.75 KiB | 990.00 KiB | 966.26 KiB |
The only way this would ever report debug is if the code was built with the DEBUG preprocessor flag. But we support building from a pre-built binary which is pre-built in release mode (I hope) so this would not work. A more reliable way is to check the entitlements to see if it's a debug build.
#skip-changelog
Closes #6990