-
Notifications
You must be signed in to change notification settings - Fork 473
For #4113. Add extension method for creating service PendingIntent
.
#4329
For #4113. Add extension method for creating service PendingIntent
.
#4329
Conversation
* @param context an [Intent] to start a service. | ||
*/ | ||
@JvmName("createPendingIntentForLaunchService") | ||
fun Intent.asPendingIntentForLaunchService(context: Context): PendingIntent = |
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.
nit: I think the name should mention that it is a foreground service!
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.
Naming ideas are welcomed. I don't think that we can use "foreground" here because it's decieving for older android versions. Technically this is just a service-that-can-start-activities-from-background.
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 think using foreground makes still sense here. It will gracefully downgrade on services before Q - similar to ContextCompat.startForegroundService()
.
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 a tough one. Both makes sense. I would lean towards adding "foreground" because most users of this extension function will be supporting O+ devices and would need to be aware that this will start a foreground service.
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.
Deal. I'll change the name, thanks. :)
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.
✔️ Done.
PendingIntent
.PendingIntent
.
…`PendingIntent`.
Codecov Report
@@ Coverage Diff @@
## master #4329 +/- ##
============================================
- Coverage 81.26% 81.26% -0.01%
+ Complexity 3993 3992 -1
============================================
Files 521 522 +1
Lines 17713 17715 +2
Branches 2607 2607
============================================
+ Hits 14395 14396 +1
- Misses 2250 2252 +2
+ Partials 1068 1067 -1
Continue to review full report at Codecov.
|
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. 🚢
bors r+ |
Build succeeded |
Moved pending intent creation for crash service to
support-utils
.Pull Request checklist
Tests: This PR includes thorough tests or an explanation of why it does notAccessibility: The code in this PR follows accessibility best practices or does not include any user facing featuresAfter merge