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
Remove application-id from macrobenchmark config file. #2683
Conversation
- update version for some dependencies - shorten the name of benchmark test class
Binary Size ReportAffected SDKsNo changes between base commit (b5d06cd) and head commit (0595e450). Test Logs
NotesHead commit (0595e450) is created by Prow via merging commits: b5d06cd 7a39540. |
Macrobenchmark ReportAffected SDKsMeasurements are for head commit (7a39540). Diffing against base commit (ff8f905) is working in progress.
|
/retest |
/retest |
1 similar comment
/retest |
dependencies: | ||
- com.google.firebase:firebase-crashlytics-ktx | ||
plugins: | ||
- com.google.firebase.crashlytics | ||
|
||
firebase-database: | ||
name: database | ||
application-id: com.google.firebase.benchmark.database | ||
dependencies: | ||
- com.google.firebase:firebase-database-ktx | ||
|
||
firebase-dynamic-links: | ||
name: dynamiclinks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No immediate action required, but what is the purpose of the name given that we already have the section already named(i.e. firebase-dynamic-links)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the beginning, I thought there can be multiple apps built for one SDK, and this name
can be used to distinguish those apps, but I suppose that is no longer the case. Right now it's used as part of the dir path for the generated test apps, but it is not necessary to have it in the path. Removing it requires some change, which I'll defer to a followup PR.
Regarding to application id, now that we only need one application id, I'm planning to add it to fireescape-integ-tests
so that I can just use preset-google-services
in prow jobs. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm
/retest |
/test check-changed |
1 similar comment
/test check-changed |
/retest |
/test check-changed |
5 similar comments
/test check-changed |
/test check-changed |
/test check-changed |
/test check-changed |
/test check-changed |
Also update version for some dependencies.