Update Gradle, Error Prone plugin, and Android Gradle Plugin #294
Conversation
This switches to the new lazy task configuration APIs
|
@msridhar @lazaroclapp thoughts? |
gradlew
Outdated
| @@ -1,7 +1,20 @@ | |||
| #!/usr/bin/env sh | |||
|
|
|||
| # added manually | |||
| ./config/hooks/install-pre-commit | |||
There was a problem hiding this comment.
Isn't this the GJF hook installer? Why are we getting rid of it?
There was a problem hiding this comment.
@lazaroclapp I just opened #297 on this. We should find a different way to do this. gradlew is an auto-generated file so this was a hack all along.
There was a problem hiding this comment.
I vote we re-add this for now, then we remove it again at the same time as we implement #297
There was a problem hiding this comment.
Sure, but then let's land #298 before this one. We really shouldn't let master be red with respect to this, hack or not :)
| @@ -31,7 +44,7 @@ APP_NAME="Gradle" | |||
| APP_BASE_NAME=`basename "$0"` | |||
|
|
|||
| # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. | |||
| DEFAULT_JVM_OPTS="" | |||
| DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' | |||
There was a problem hiding this comment.
This seems very very low, but if it works for building NullAway, then all the better, I suppose.
| option("NullAway:AnnotatedPackages", "com.uber") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Have we tried manually injecting a null dereference on the sample app and seeing that NullAway does trigger on it? (e.g. uncomment https://github.com/uber/NullAway/blob/master/sample-app/src/main/java/com/uber/myapplication/MainActivity.java#L33 do .\gradlew build, observe error).
I am not sure we have integration tests for this config, so is worth doing that small manual sanity check :)
There was a problem hiding this comment.
Hey @ZacSweers, would you mind rebasing and testing this? After that I'd be glad to merge this.
Edit: Actually, never mind, testing it now :)
|
To clarify master won’t turn red unless we land a PR with bad formatting.
We still check for proper formatting on Travis independent of the commit
hook. Without this gradlew hack GJF needs to be run manually
…On Fri, Apr 5, 2019 at 8:49 AM Lázaro Clapp ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In gradlew
<#294 (comment)>:
> @@ -1,7 +1,20 @@
#!/usr/bin/env sh
-# added manually
-./config/hooks/install-pre-commit
Sure, but then let's land #298 <#298>
before this one. We really shouldn't let master be red with respect to
this, hack or not :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#294 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AALyUYO17l814vOrb5Wnobv6_aCJ-fx-ks5vd3CmgaJpZM4cVTub>
.
|
I forgot about the CI check. That makes it less of an issue. Still, I feel is better if we guarantee that checking out the repo and building at any commit results on the git hook being installed, but "master will be red" is definitely an exaggeration there :) Either way, I think we can land #298 , then just merge this (preferring this version of |
|
Yup sounds good!
On Fri, Apr 5, 2019 at 9:11 AM Lázaro Clapp <notifications@github.com>
wrote:
… To clarify master won’t turn red unless we land a PR with bad formatting.
We still check for proper formatting on Travis independent of the commit
hook. Without this gradlew hack GJF needs to be run manually
… <#m_-2327111134865884889_>
On Fri, Apr 5, 2019 at 8:49 AM Lázaro Clapp ***@***.***> wrote: ***@***.****
commented on this pull request. ------------------------------ In gradlew <#294
(comment)
<#294 (comment)>>: > @@
-1,7 +1,20 @@ #!/usr/bin/env sh -# added manually
-./config/hooks/install-pre-commit Sure, but then let's land #298
<#298> <#298
<#298>> before this one. We really
shouldn't let master be red with respect to this, hack or not :) — You are
receiving this because you were mentioned. Reply to this email directly,
view it on GitHub <#294 (comment)
<#294 (comment)>>, or
mute the thread
https://github.com/notifications/unsubscribe-auth/AALyUYO17l814vOrb5Wnobv6_aCJ-fx-ks5vd3CmgaJpZM4cVTub
.
I forgot about the CI check. That makes it less of an issue. Still, I feel
is better if we guarantee that checking out the repo and building at any
commit results on the git hook being installed, but "master will be red" is
definitely an exaggeration there :)
Either way, I think we can land #298
<#298> , then just merge this
(preferring this version of gradlew anyways), and we will have guaranteed
that the commit hook is added when building every commit.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#294 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AALyUaC7JYJEi9zIqIvosSYKk6HmWnSWks5vd3WbgaJpZM4cVTub>
.
|
|
@lazaroclapp could you merge this and #295 assuming they are ok with you? I think they are all set but don't want to mess something up |
Modernizes the android example with the new lazy task configuration APIs. Based on implementation figured out here: uber/AutoDispose#330