Skip to content

Fix API text files for App Distribution #3232

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

Closed
wants to merge 3 commits into from

Conversation

rachaprince
Copy link

Based off #3213 . Fixes both the java and kotlin api text files by running

/gradlew :firebase-app-distribution:generateApiTxtFile and
/gradlew :firebase-app-distribution:ktx:generateApiTxtFile

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Dec 10, 2021

Coverage Report

Affected SDKs

No changes between base commit (8bc2ab4) and head commit (28e4b040).

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (28e4b040) is created by Prow via merging commits: 8bc2ab4 6679cca.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Dec 10, 2021

Binary Size Report

Affected SDKs

No changes between base commit (8bc2ab4) and head commit (28e4b040).

Test Logs

Notes

Head commit (28e4b040) is created by Prow via merging commits: 8bc2ab4 6679cca.

@lfkellogg
Copy link
Contributor

A couple questions:

  • Should we wait until Manny's PR is merged so there will be fewer public classes/methods to document?
  • I'm guessing we need to update this periodically as we change the public surface of our API. How will we remember to do this next time?

@rachaprince
Copy link
Author

rachaprince commented Dec 10, 2021

Good callout, let's wait for Manny's PR to land.

The second question is also a good one. It seems like the api-information test should run on every PR. I wonder if there is a configuration setting that should be fixed in this repo. I'll ask a question to the Firebase Android Core team

@ehsannas
Copy link
Contributor

Thanks @rachaprince for fixing this. I didn't have time to work on #3213 any further. I'll close that.

@rachaprince
Copy link
Author

Looking at the public api files generated by the script, I think there are still some places where we will want to reduce the visibility of methods and classes

@rachaprince
Copy link
Author

Closing this in favor of #3235, which limits the number of public classes and methods

@rachaprince rachaprince deleted the fad-fix-api-text-files branch January 12, 2022 20:55
@firebase firebase locked and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants