Skip to content
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

Use RequiresApi instead of TargetApi in NotificationManager #3265

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

rvandermeulen
Copy link
Contributor

@rvandermeulen rvandermeulen commented Dec 3, 2024

AGP 8.9.0 is tightening up the lint checks around TargetApi/RequiresApi. With the most recent alpha04 build, we see the following error:

/reference-browser/app/src/main/java/org/mozilla/reference/browser/NotificationManager.kt:181: Error: Use @RequiresApi(Build.VERSION_CODES.O) instead of @TargetApi` to propagate the requirement to callers of createNotificationChannelIfNeeded [UseRequiresApi]
    @TargetApi(Build.VERSION_CODES.O)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

   Explanation for issues of type "UseRequiresApi":
   The @TargetApi annotation only suppresses API warnings locally.
   @RequiresApi on the other hand will propagate the requirement out to any
   callers of this API, making sure that they either perform API level checks
   (using for example SDK_INT), or defining @RequiresApi annotations
   themselves.

   (The @TargetApi annotation predates @RequiresApi, and was introduced as an
   early way to suppress lint API warnings for a particular API level.
   Accidentally using @TargetApi can lead to crashes since there is no check
   that other calls to this method actually check that the call is safe.)

@rvandermeulen rvandermeulen added the needs landing Auto lands approved and green PRs. label Dec 3, 2024
@rvandermeulen
Copy link
Contributor Author

And yes, m-c is going to require some similar changes.

Copy link
Collaborator

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for answering my question before letting me ask it. 😄

run-minority-report-3220321037

@mergify mergify bot merged commit c3cc251 into master Dec 3, 2024
15 checks passed
@rvandermeulen rvandermeulen deleted the rvm/requiresapi branch December 3, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs landing Auto lands approved and green PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants