Skip to content

[improve][pip]pip-417:Enable SpotBugs FindPublicAttributes Check #24290

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

3pacccccc
Copy link
Contributor

PIP documentation.
Implementation PR: #24285

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added PIP doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. labels May 12, 2025
@crossoverJie
Copy link
Member

I also have doubts about extensively using public fields, but I feel the cost of making changes is not proportional to the benefits.

@BewareMyPower
Copy link
Contributor

but I feel the cost of making changes is not proportional to the benefits.

+1 to this point. @3pacccccc Could you have a demonstration for how many interfaces and classes will be affected?

@lhotari
Copy link
Member

lhotari commented May 13, 2025

but I feel the cost of making changes is not proportional to the benefits.

+1 to this point. @3pacccccc Could you have a demonstration for how many interfaces and classes will be affected?

@BewareMyPower the changes can be seen in #24285

@BewareMyPower
Copy link
Contributor

I see. It LGTM now.

@3pacccccc
Copy link
Contributor Author

I see. It LGTM now.
@BewareMyPower But I worry about this change will break current public interfaces or internal interfaces, any suggestions?

@BewareMyPower
Copy link
Contributor

IMO, it's okay if these interfaces changes only affect the plugins. Otherwise, it would be very complicated to maintain the compatibility and the code would become very ugly. I started a discussion before: https://lists.apache.org/thread/v4bswc3byqtvpl3h4cjpvzm8sbsy0wq9

And I agree with this point

It would essentially prevent most
refactorings and improvements to Pulsar. We could choose to do so, but
it has consequences.
Each class should have a reason why it's part of the stable API. It
shouldn't be too hard to list what is used by custom plugins and why.
Custom plugin authors should provide this information and this could
be taken into account when the stable API annotations are added and
reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. PIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants