Migrated new APK size tool to OSS repository #74
Conversation
|
/test check |
1 similar comment
|
/test check |
| * | ||
| * <p>This may be any type recognized by Gradle as a file. The format of the file's contents is | ||
| * headerless CSVwith a colon as a delimiter: projectName_buildVariant:sdkId. The first column | ||
| * contains both the project name and build variant separated by an hyphen, {@code |
There was a problem hiding this comment.
I think a multiline example of what this would look like in practice for firebase-android-sdk would be helpful
firestore_debug:1
database_debug:1
There was a problem hiding this comment.
Done. This is actually supposed to be a hyphen. It works for now, but we may need to change it in the future if we start have hyphens in project and build variant names.
buildSrc/src/main/groovy/com/google/firebase/gradle/GenerateMeasurementsTask.groovy
Show resolved
Hide resolved
| * firestore-aggressive}, for example. | ||
| */ | ||
| @InputFile | ||
| File sdkMap |
There was a problem hiding this comment.
rename to sdkMapFile ?
|
|
||
| private def createSdkMap() { | ||
| def map = [:] | ||
| def path = project.file(sdkMap) |
There was a problem hiding this comment.
you should be able to just say sdkMap.eachLine
There was a problem hiding this comment.
Ahh yes. Originally the inputs and outputs were objects, and I relied on Gradle converting them to files. This enables strings and other types to be used in the Gradle files.
Vlad and I decided we didn't need this flexibility, but there are still some artifacts of it hanging around in the code.
buildSrc/src/main/groovy/com/google/firebase/gradle/GenerateMeasurementsTask.groovy
Show resolved
Hide resolved
| @@ -0,0 +1,64 @@ | |||
| { | |||
There was a problem hiding this comment.
We should delete this
allisonbm92
force-pushed
the
allisonbm-newsizetool
branch
from
620316b to
a4213bd
Compare
|
/test connected-check |
|
/test smoke-tests-debug |
allisonbm92
force-pushed
the
allisonbm-newsizetool
branch
from
a4213bd to
60419cd
Compare
|
/test build-plugins-check |
| def report = createReport(sizes) | ||
| // Check if we need to run human-readable or upload mode. | ||
| if (project.hasProperty("pull_request")) { | ||
| def pullRequestNumber = project.properties["pull_request"] |
There was a problem hiding this comment.
Fixed. My editor seems to be throwing tabs in... :(
| reportFile.withWriter { | ||
| it.write(report) | ||
| } | ||
| private def calculateSizesForHumanReadable(variants) { |
There was a problem hiding this comment.
Maybe rename to calculateHumanReadableSizes?
There was a problem hiding this comment.
Thanks, naming isn't always my strong suit.
|
I updated the README (thanks for catching that), and I'll wait for the next pull requests to add the tests, because we're in the process of switching to JSON from protocol buffers. |
|
/test connected-check |
| </style> | ||
| --> | ||
|
|
||
| <!-- Base application theme. --> |
There was a problem hiding this comment.
Nit: Can we get rid of this and use the default app compat theme?
| import com.google.firebase.gradle.GenerateMeasurementsTask | ||
| import com.google.firebase.gradle.UploadMeasurementsTask | ||
|
|
||
| // Linting needs to be disabled as none of these apps are real or intended to conform to |
There was a problem hiding this comment.
Can we add a Github issue to enable this?
No description provided.