Skip to content

Conversation

@zhztheplayer
Copy link
Member

What changes were proposed in this pull request?

Remove a redundant @VisibleForTesting on test code TestMemoryConsumer#freePage (added in #21570).

Why are the changes needed?

Test code doesn't need this annotation.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

Was this patch authored or co-authored using generative AI tooling?

No.

@zhztheplayer zhztheplayer marked this pull request as ready for review July 31, 2025 07:57
@github-actions github-actions bot added the CORE label Jul 31, 2025
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for making a PR, @zhztheplayer .

However, the annotation is proper and correct.

This annotation is supposed to help a developer understand the reason why this is public instead of protected when TestMemoryConsumer extends MemoryConsumer.

protected void freePage(MemoryBlock page) {

@zhztheplayer
Copy link
Member Author

Hi @dongjoon-hyun,

Thank you for your insights!

Just thought the annotation was unintentionally added, as it's a little bit counter-intuitive because it appears in test code.

I just found some discussion about this flag in #21570 also. Since it means there was a reason from the author, I will be good to keep it. :)

@dongjoon-hyun
Copy link
Member

Thank you for closing your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants