Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Fixes #9832 - Change targetSdkVersion to Android 10 #11014

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

NotWoods
Copy link
Contributor

No major changes to upgrade now!

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

After merge

  • Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

@NotWoods NotWoods requested review from grigoryk and jonalmeida May 28, 2020 22:16
@NotWoods NotWoods force-pushed the android-10 branch 3 times, most recently from 0ee30b7 to 2443a94 Compare June 12, 2020 18:21
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2020

Codecov Report

Merging #11014 into master will decrease coverage by 13.30%.
The diff coverage is 14.28%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #11014       +/-   ##
=============================================
- Coverage     22.12%    8.82%   -13.31%     
+ Complexity      725      245      -480     
=============================================
  Files           377      373        -4     
  Lines         15098    15040       -58     
  Branches       1957     2032       +75     
=============================================
- Hits           3341     1327     -2014     
- Misses        11469    13619     +2150     
+ Partials        288       94      -194     
Impacted Files Coverage Δ Complexity Δ
...ozilla/fenix/library/bookmarks/BookmarkFragment.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...mozilla/fenix/library/history/HistoryController.kt 3.84% <0.00%> (-96.16%) 0.00 <0.00> (ø)
...a/fenix/settings/logins/SavedLoginsAuthFragment.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...n/java/org/mozilla/fenix/utils/ClipboardHandler.kt 0.00% <0.00%> (-73.34%) 0.00 <0.00> (-14.00)
...java/org/mozilla/fenix/whatsnew/WhatsNewStorage.kt 0.00% <ø> (-94.12%) 0.00 <0.00> (ø)
...illa/fenix/library/bookmarks/BookmarkController.kt 95.83% <50.00%> (-4.17%) 0.00 <0.00> (ø)
...pp/src/main/java/org/mozilla/fenix/ext/Activity.kt 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...pp/src/main/java/org/mozilla/fenix/ext/Drawable.kt 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...src/main/java/org/mozilla/fenix/ext/ImageButton.kt 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (ø%)
...rc/main/java/org/mozilla/fenix/GlobalDirections.kt 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-2.00%)
... and 162 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d76d8d1...02533af. Read the comment docs.

Copy link
Contributor

@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.

This looks good to me now that we've cut a release.

private fun showPinVerification() {
val manager = activity?.getSystemService(KEYGUARD_SERVICE) as KeyguardManager
val manager = activity?.getSystemService<KeyguardManager>()!!
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should use un-safe calls if the newer target SDK asserts them as nullable. Some devices maybe not support some system services and we shouldn't crash because of that.

Copy link
Contributor Author

@NotWoods NotWoods Jul 8, 2020

Choose a reason for hiding this comment

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

We're already using unsafe calls as we cast nullable to not-null. The SDK doesn't assert if a system service is null or not, the extension method simply always returns a nullable type.

@@ -16,7 +17,7 @@ private const val MIME_TYPE_TEXT_HTML = "text/html"
* A clipboard utility class that allows copying and pasting links/text to & from the clipboard
*/
class ClipboardHandler(context: Context) {
private val clipboard = context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager
private val clipboard = context.getSystemService<ClipboardManager>()!!
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the previous comment.

@NotWoods NotWoods merged commit c99a8f5 into mozilla-mobile:master Jul 9, 2020
@NotWoods NotWoods deleted the android-10 branch July 9, 2020 17:50
@liuche liuche mentioned this pull request Jul 20, 2020
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants