Skip to content
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

Use Android linter from cmdline-tools #28153

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Conversation

blasten
Copy link

@blasten blasten commented Aug 18, 2021

Use the Android linter from cmdline-tools.
We are currently using the one from tools/bin/lint which is very old, and doesn't work with Java 11.

@@ -106,14 +104,13 @@ Future<int> runLint(ArgParser argParser, ArgResults argResults) async {
if (html) {
lintArgs.addAll(<String>['--html', argResults['out'] as String]);
}
final String? javaHome = await getJavaHome();
final String javahome = getJavaHome(inArgument);
print('Using JAVA_HOME=$javahome');
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is this print meant to be here?

Copy link
Author

Choose a reason for hiding this comment

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

it's intended. It helps inform the user of this script that this ENV variable is set. Particularly, relevant in CI.

@@ -43,7 +43,7 @@
* The most obvious example of when this may come in handy is if an application wishes to subclass
* the Android v4 support library's {@code FragmentActivity}.
*
* <p><b>Usage:</b></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why the closing

was removed?

Copy link
Author

Choose a reason for hiding this comment

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

The closing tag </p> isn't required. See this thread google/google-java-format#61

Copy link
Contributor

@GaryQian GaryQian left a comment

Choose a reason for hiding this comment

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

A few nits, LGTM

@blasten blasten added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants