-
-
Notifications
You must be signed in to change notification settings - Fork 335
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
Don’t force cast to NSComparisonPredicate
in TERNARY operator
#4232
Conversation
NSComparisonPredicate
NSComparisonPredicate
NSComparisonPredicate
in tenary operator
NSComparisonPredicate
in tenary operatorNSComparisonPredicate
in TERNARY operator
…y-cocoa into fix/swift-data-predicate
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.
Thank you @denrase 💯. LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4232 +/- ##
=============================================
- Coverage 91.445% 91.375% -0.070%
=============================================
Files 611 610 -1
Lines 49214 49117 -97
Branches 17781 17698 -83
=============================================
- Hits 45004 44881 -123
- Misses 4116 4143 +27
+ Partials 94 93 -1
... and 22 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2af280d | 1226.46 ms | 1229.81 ms | 3.35 ms |
17afc4b | 1228.94 ms | 1251.10 ms | 22.16 ms |
1437c68 | 1223.94 ms | 1249.76 ms | 25.82 ms |
60d6cec | 1257.14 ms | 1273.92 ms | 16.78 ms |
19b93f9 | 1212.46 ms | 1226.86 ms | 14.40 ms |
6001822 | 1234.49 ms | 1265.20 ms | 30.71 ms |
d80d410 | 1231.50 ms | 1240.14 ms | 8.64 ms |
01a28a9 | 1245.43 ms | 1255.98 ms | 10.55 ms |
33b2f51 | 1251.24 ms | 1259.18 ms | 7.94 ms |
b6ba04e | 1217.45 ms | 1248.92 ms | 31.47 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2af280d | 20.76 KiB | 435.22 KiB | 414.46 KiB |
17afc4b | 20.76 KiB | 436.25 KiB | 415.49 KiB |
1437c68 | 22.85 KiB | 410.96 KiB | 388.11 KiB |
60d6cec | 22.84 KiB | 403.51 KiB | 380.67 KiB |
19b93f9 | 21.58 KiB | 694.46 KiB | 672.88 KiB |
6001822 | 22.85 KiB | 410.98 KiB | 388.13 KiB |
d80d410 | 20.76 KiB | 425.71 KiB | 404.95 KiB |
01a28a9 | 22.85 KiB | 405.39 KiB | 382.55 KiB |
33b2f51 | 20.76 KiB | 432.17 KiB | 411.41 KiB |
b6ba04e | 20.76 KiB | 414.45 KiB | 393.69 KiB |
There are some test failures, but it's not obvious to me how they relate to this PR. For example
Are these just flaky @philipphofmann ? |
📜 Description
Tenary operators can contain any type of predicate, not just comparison. Removed the force cast and called the parent method for the description.
💡 Motivation and Context
Fixes #3553
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.