Closed Bug 1284544 Opened 3 years ago Closed 3 years ago

[lint] OldTargetApi: Target SDK attribute is not targeting latest version

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

defect
Not set

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: ahunt, Assigned: ahunt)

References

Details

Attachments

(3 files)

Sdk 24 has now been released, we're building against 23.

Any development machine with the 24 sdk will therefore complain with the following lint error:

OldTargetApi: Target SDK attribute is not targeting latest version
../generated/source/preprocessed_manifest/AndroidManifest.xml:10: Not targeting the latest versions of Android; compatibility modes apply. Consider testing and updating this version. Consult the android.os.Build.VERSION_CODES javadoc for details.

We should probably suppress this warning, since we can track following the sdk updates separately (and we don't want to break everyone's lint in the meantime)
There's also a second error, which seems somewhat useless to me:

 ../../../../../../../mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:458: This method is not overriding anything with the current build target, but will in API level 24 (current target is 23): org.mozilla.gecko.GeckoInputConnection#getHandler

 455     }
 456 
 457     // Android N: @Override // InputConnection
 458     public Handler getHandler() {
 459         if (isPhysicalKeyboardPresent()) {
 460             return ThreadUtils.getUiHandler();
Review commit: https://reviewboard.mozilla.org/r/62400/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62400/
Attachment #8768049 - Flags: review?(michael.l.comella)
Attachment #8768050 - Flags: review?(michael.l.comella)
Attachment #8768051 - Flags: review?(michael.l.comella)
We're still building with the 23 sdk, but lint is now giving us errors based
on the 24 sdk. We therefore need to explicitly suppress the error (even though
in reality we don't want to suppress the error so that we don't miss adding the
@Override annotation when we switch to 24).

Review commit: https://reviewboard.mozilla.org/r/62402/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62402/
GET_SERVICES is already implicit in this method name. Using lint running against
sdk 24 we get a warning that GET_SERVICES isn't a permitted flag. (Previous sdks
didn't yet contain a list of allowable flags.)

Review commit: https://reviewboard.mozilla.org/r/62404/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62404/
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Comment on attachment 8768049 [details]
Bug 1284544 - lint: don't error on "OldTargetSdk", we don't want to upgrade the sdk yet

https://reviewboard.mozilla.org/r/62400/#review59584
Attachment #8768049 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8768050 [details]
Bug 1284544 - lint: suppress useless override error caused by presence of 24 sdk

https://reviewboard.mozilla.org/r/62402/#review59586

::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:459
(Diff revision 1)
>          final Configuration config = v.getContext().getResources().getConfiguration();
>          return config.keyboard != Configuration.KEYBOARD_NOKEYS;
>      }
>  
>      // Android N: @Override // InputConnection
> +    // We need to suppress lint complaining about the lack override here in the meantime: it wants us to build

nit: it'd be good to file a bug and cite a bug number to fix this.

::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:461
(Diff revision 1)
>      }
>  
>      // Android N: @Override // InputConnection
> +    // We need to suppress lint complaining about the lack override here in the meantime: it wants us to build
> +    // against sdk 24, even though we're using 23, and therefore complains about the lack of override.
> +    // Once we update to 24, we can use the actual ovveride annotation and remove the lint suppression.

nit: override
Attachment #8768050 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8768051 [details]
Bug 1284544 - Remove redundant flag from PackageManager.queryIntentServices

https://reviewboard.mozilla.org/r/62404/#review59588

Android API docs agree that this is not a valid flag: https://developer.android.com/reference/android/content/pm/PackageManager.html#queryIntentServices(android.content.Intent,%20int)

I'm slightly concerned there could be an unexpected behavior change but I'm okay listening to the redundancy in the docs & linter. :)
Attachment #8768051 - Flags: review?(michael.l.comella) → review+
Blocks: 1287651
(In reply to Michael Comella (:mcomella) from comment #6)
> Comment on attachment 8768050 [details]
> Bug 1284544 - lint: suppress useless override error caused by presence of 24
> sdk
> 
> https://reviewboard.mozilla.org/r/62402/#review59586
> 
> ::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:459
> (Diff revision 1)
> >          final Configuration config = v.getContext().getResources().getConfiguration();
> >          return config.keyboard != Configuration.KEYBOARD_NOKEYS;
> >      }
> >  
> >      // Android N: @Override // InputConnection
> > +    // We need to suppress lint complaining about the lack override here in the meantime: it wants us to build
> 
> nit: it'd be good to file a bug and cite a bug number to fix this.
> 

Filed bug 1287651 for that!
https://hg.mozilla.org/integration/fx-team/rev/0cb14ba581524b331fd3f0c850695a963c6cc3e4
Bug 1284544 - lint: don't error on "OldTargetSdk", we don't want to upgrade the sdk yet r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/5585aaffb2b674068cad9a125a7528a57c0962c0
Bug 1284544 - lint: suppress useless override error caused by presence of 24 sdk r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/a35a15008f0c37488cfc5f907f6d553aab028eb8
Bug 1284544 - Remove redundant flag from PackageManager.queryIntentServices r=mcomella
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 50 → mozilla50
You need to log in before you can comment on or make changes to this bug.