-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Add immutable flag to pending intents #593
Conversation
ce073f1
to
3160f2f
Compare
) | ||
return NotificationCompat.Action(R.drawable.chucker_ic_delete_white, clearTitle, intent) | ||
} | ||
|
||
fun dismissNotifications() { | ||
notificationManager.cancel(TRANSACTION_NOTIFICATION_ID) | ||
} | ||
|
||
private fun immutableFlag() = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { |
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.
Do we really need this check?
Flags are generally forwards compatible.
During compilation constant will be inlined and in runtime on pre-M those bits won't be read.
Warning will be generated about using constants from higher API levels than min SDK but in this case such usage is legitimate IMHO.
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.
I'm not sure what are you proposing. I don't want IDE to highlight this and Lint to fail. Do you suggest to add @SuppressLint("InlinedApi")
? R8 can remove this condition anyway with -assumevalues
. Then there is dex2oat and finally ART itself.
Though I don't mind adding annotation. Depends on what others think.
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.
Do you suggest to add @SuppressLint("InlinedApi")
Exactly.
R8 can remove this condition
True, but not everyone use R8.
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.
True, but not everyone use R8.
I would say that if someone does not use R8 then they don't care about optimizations anyway. :)
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.
I'd be more for having the explicit if-then-else. It's more aligned with the rest of the codebase and makes the code more explicit.
* Update all dependencies and configs, update code for Detekt * Prepare for next release (#494) * Prepare for next release * Update workflows and readme to mirror latest changes * Update publishing action name (#495) * Fixed typo on DialogData (#501) * Update README.md (#505) fix the typo * Fix setting request body plain text in transaction (#538) * Fix setting request body plain text in transaction * Add test for plain text request body * Switch to CircularProgressIndicator * Switch to Activity Result API * Add immutable flag to pending intents (#593) * Update Github Actions workflows to match latest ones * Bump version * Resolve lint issues * Remove test using newer OkHttp API * Remove breaking change with BuildConfig removal * Bump kotlinVersion from 1.5.10 to 1.5.20 (#639) Bumps `kotlinVersion` from 1.5.10 to 1.5.20. Updates `kotlin-gradle-plugin` from 1.5.10 to 1.5.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v1.5.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.5.10...v1.5.20) Updates `kotlin-stdlib` from 1.5.10 to 1.5.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v1.5.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.5.10...v1.5.20) --- updated-dependencies: - dependency-name: org.jetbrains.kotlin:kotlin-gradle-plugin dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-stdlib dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Nicola Corti <corti.nico@gmail.com> Co-authored-by: Okan AYDIN <okanaydin1994@gmail.com> Co-authored-by: Michał Sikora <michalsikora90@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
📄 Context
Android 31 requires to specify mutability of pending intents.
📝 Changes
I added mutability flag to our pending intents.
🚫 Breaking
No.
🛠️ How to test
Not sure, run on Android 31?
⏱️ Next steps
Either cherry–pick it to
3.x
or plan release of4.x
soonish.