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

Convert more classes to CompileStatic #569

Merged
merged 2 commits into from
Jun 24, 2022

Conversation

ejona86
Copy link
Collaborator

@ejona86 ejona86 commented Jun 24, 2022

As a solution to use AnimalSniffer to detect incompatibilities with
Java 8, as discussed at
#565 (comment)

CC @rougsig

As a solution to use AnimalSniffer to detect incompatibilities with
Java 8, as discussed at
google#565 (comment)
@ejona86 ejona86 requested a review from YifeiZhuang June 24, 2022 17:28
@@ -415,6 +418,7 @@ public abstract class GenerateProtoTask extends DefaultTask {
return flavors
}

@TypeChecked(TypeCheckingMode.SKIP) // Don't depend on AGP
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to add facade on SourceSet in some reasons:
1 better code, no Object
2 agp contains two different classes for sourceSets, deprecated one and the new one. Its the main reason why plugin doesnt work with newer agp.
3 a bit easy support to kmp

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have little issue with that, although it seems best to do that separately. Right now it is "get as much compiling static as easily possible." This is a trivial change and gets us closer to Java 11, if not AGP. Over time we increase what is compiled static and clean AGP handling (and fix AGP).

@rougsig
Copy link
Collaborator

rougsig commented Jun 24, 2022

@ejona86 I will add animal sniffer plugin tomorrow. Or will you do it?

@ejona86
Copy link
Collaborator Author

ejona86 commented Jun 24, 2022

It doesn't really matter to me who does it. I had hoped to do a pass of ProtobufPlugin to CompileStatic since it's a rather important class. But looking, it seems maybe ¾ of the class can't be converted because it mixes in Android usages. I guess we'll just leave that for later.

@ejona86 ejona86 merged commit 2758817 into google:master Jun 24, 2022
@ejona86 ejona86 deleted the moar-compile-static branch June 24, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants