-
Notifications
You must be signed in to change notification settings - Fork 129
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
RUM-3808 fix: Avoid name collision with Required Reason APIs #1774
RUM-3808 fix: Avoid name collision with Required Reason APIs #1774
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 2727 Passed, 0 Skipped, 5m 51.73s Wall Time 🔻 Code Coverage Decreases vs Default Branch (8)
|
private var userDefaults: UserDefaults | ||
private var userDefaults: UserDefaults // swiftlint:disable:this required_reason_api_name |
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.
💡 This is true positive - UserDefaults
is the only Required Reason API we use. It is declared in our manifest.
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.
Thanks!!
@@ -89,6 +89,61 @@ custom_rules: | |||
- doccomment | |||
message: "`UIApplication.shared` is unavailable in some environments. Check `UIApplication.managedShared`." | |||
severity: error | |||
required_reason_api_name: # prevents from declaring symbols that may conflict with Apple's Required Reason APIs |
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.
perfect!
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.
Nice!
What and why?
📦 This PR renames symbols in our codebase to avoid false-positive warnings on using Required Reason APIs. Next to this, it adds basic linter rule to prevent from using names that may result with conflicts.
Related GH Issues: #1749, #1756
How?
Renamed:
.creationDate
→.fileCreationDate
andsystemUptime
→getSystemUptime
.Added custom linter rule that scans the code for Required Reason API names. If found, it raises an error:
The rule is inspired by the text scanner from this repo. It is up to date with the list of Apple APIs.
Review checklist
Custom CI job configuration (optional)
tools/