[android] Add lint support

[Guardiola31337]

Fixes #8357

• Adds lint using our current baseline as starting point

👀 @zugaldia @tobrun

[tobrun]

This is great! Are these rules also enforced on CI? or is that one bridge too far?

[zugaldia]

While I'm happy to merge this PR with the current baseline, by looking at the SDK file, it seems to me that there're many issues that are easily fixable (e.g. setting a default Locale during logging, or using @SuppressWarnings({"MissingPermission"}) when we're sure we don't need to check for a permission).

@Guardiola31337 Could you open a follow up ticket where we'll review the current baseline to make it as small as possible? This looks like a great starter task.

[Guardiola31337]

@tobrun

Are these rules also enforced on CI?

Not yet, we should add a lint check step. I commented how in #8357 (comment) 👇

In order to accomplish this, we could add a lint check on CI. Run lint with Gradle.

@zugaldia

While I'm happy to merge this PR with the current baseline, by looking at the SDK file, it seems to me that there're many issues that are easily fixable (e.g. setting a default Locale during logging, or using @SuppressWarnings({"MissingPermission"}) when we're sure we don't need to check for a permission).

Could you open a follow up ticket where we'll review the current baseline to make it as small as possible? This looks like a great starter task.

Wasn't my intention, actually we could work here and in the meantime we could add lint on CI.
Cool?

[zugaldia]

we could work here and in the meantime we could add lint on CI.

Yeah, let's add lint to CI now with the current baseline. We can then work (separate PR?) on reducing the baseline. That way lint doesn't have to wait for us to work on the baseline.

[tobrun]

Executing lint from command line is something as:

./gradlew lint 

Though we could also expose a separate make target for it as we now do for checkstyle.

[Guardiola31337]

👀 ready for review @tobrun @zugaldia

[zugaldia]

@Guardiola31337 Why do we have different baselines for local dev and for CI? Shouldn't they both be the same?

[Guardiola31337]

@zugaldia they should, but in practice baselines include absolute paths (pointing to the files containing an error) which are different in local and in CI, so we need to keep both 😞

[Guardiola31337]

@zugaldia at the moment, if you run this locally and remove any error, you'll get a warning from the command line advising you to remove it from the baseline. If you removed it you should remove it too from the CI baseline (which is the only one included in the repo). Eventually, it'll be removed (when we remove all current lint errors included).

[zugaldia]

@Guardiola31337 Thanks for the explanation 👍 . Could we add a big note to the XML lint files saying that? Something like "remember to update file XXXX.XML as well if you make any changes to this file".

[Guardiola31337]

@zugaldia I've added the following comment

<!-- REMEMBER! First you run Lint locally you'll need to move the lint-baseline.xml file
    generated into the lint folder and called it lint-baseline-local.xml
    If you remove any error when running Lint locally, you'll get a warning from the command
    line advising you to remove it from the baseline. If you remove it (remember to remove it
    from lint-baseline-local.xml file) you should remove it too from
    lint-baseline-ci.xml (THIS FILE) which is the only one included in the repo.
    Eventually, it'll be removed (when we remove all current lint errors included). -->

into the different lint-baseline-ci.xml files.

[zugaldia]

@Guardiola31337 👍