Skip to content

Cleaning up unnecessary use of "@hide annotations" (b/177448055) #2544

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

Merged
merged 3 commits into from
Mar 29, 2021

Conversation

ramanpreetSinghKhinda
Copy link
Contributor

@ramanpreetSinghKhinda ramanpreetSinghKhinda commented Mar 24, 2021

Few Points:

  • We should be using @hide javadoc annotation to hide certain packages/classes/methods (which are declared public) to be considered a part of public API.

  • Double use of @hide javadoc annotation is not required. This was a requirement with GmsCore (go/gmscore-hide-annotation-lsc). Also that second annotation was @Hide Java annotation (which is a GmsCore annotation) and not @hide javadoc annotation (which is a general javadoc annotation) which seems like happened during code migration to GitHub.

  • This annotation should only be used in public classes/methods.

  • Adding this annotation to the method will hide that method.

  • Adding this annotation to the class level will hide the entire class and its public methods.

  • Adding this annotation to the package declaration in thepackage-info.java class makes the entire classes within that package to be hidden. Note that this does not work transitively for child packages and so each child package would also need there own package-info.java class.

  • There's no way currently to not hide some of the classes/methods when the entire package is marked hidden. So in that case where a package contains some classes/methods which has to be public than we have to apply this annotation class/method wise.

What does hiding means?

  • No javadoc will be generated for that package/class/method
  • api.txt won't get updated for that package/class/method

FAQs

  • Does this PR trigger any public api change?
    No, I have already validated everything by running the below command and no change was detected (caveat: sometimes you have to touch the api.txt file to trigger the run).
firebase-android-sdk$./gradlew firebase-perf:generateApiTxtFile

WANT_LGTM = @vkryachko

@googlebot googlebot added the cla: yes Override cla label Mar 24, 2021
@ramanpreetSinghKhinda ramanpreetSinghKhinda changed the title Cleaning up unnecessary use of "@hide annotations" Cleaning up unnecessary use of "@hide annotations" (b/177448055) Mar 24, 2021
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 24, 2021

Coverage Report

Affected SDKs

No changes between base commit (39ce055) and head commit (31b0513b).

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 (31b0513b) is created by Prow via merging commits: 39ce055 cdaa292.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 24, 2021

Binary Size Report

Affected SDKs

  • firebase-perf

    Type Base (39ce055) Head (31b0513b) Diff
    aar 304 kB 304 kB +29 B (+0.0%)
    apk (release) 2.43 MB 2.43 MB +72 B (+0.0%)

Test Logs

Notes

Head commit (31b0513b) is created by Prow via merging commits: 39ce055 cdaa292.

Copy link
Contributor

@jeremyjiang-dev jeremyjiang-dev left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed PR description, which makes my review very easy! I recently merged FirebasePerformanceInitializer.java and AppStateMonitor.java, could you do a sanity check there?

@ramanpreetSinghKhinda
Copy link
Contributor Author

Merge this PR after #2545.

Reason: #2546 (comment)

@ramanpreetSinghKhinda
Copy link
Contributor Author

/test smoke-tests

@ramanpreetSinghKhinda ramanpreetSinghKhinda merged commit d18cc40 into master Mar 29, 2021
@ramanpreetSinghKhinda ramanpreetSinghKhinda deleted the perf-annotations branch March 29, 2021 20:09
@firebase firebase locked and limited conversation to collaborators Apr 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Override cla size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants