Skip to content

[cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly#19513

Merged
AnonHxy merged 1 commit intoapache:masterfrom
StevenLuMT:master_cleanCode
Feb 16, 2023
Merged

[cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly#19513
AnonHxy merged 1 commit intoapache:masterfrom
StevenLuMT:master_cleanCode

Conversation

@StevenLuMT
Copy link
Copy Markdown
Member

@StevenLuMT StevenLuMT commented Feb 14, 2023

Motivation

Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Modifications

Cleanup ManagedLedgerImpl.isLedgersReadonly.

Verifying this change

  • Make sure that the change passes the CI checks.
    This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

Check the box below or label this PR directly.

Need to update docs?

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

Matching PR in forked repository

PR in forked repository: StevenLuMT#5

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Feb 14, 2023
@AnonHxy AnonHxy added the type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use label Feb 15, 2023
@AnonHxy AnonHxy added this to the 3.0.0 milestone Feb 15, 2023
@StevenLuMT
Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

@AnonHxy AnonHxy merged commit 89c1de3 into apache:master Feb 16, 2023
@michaeljmarshall
Copy link
Copy Markdown
Member

@StevenLuMT - what is the justification for removing this method? We added it as part of PIP 180. I think we should consult with @Jason918 before we do this kind of cleanup up, as this feature was only just added in November with #18265 and might not be something we should remove.

@Jason918
Copy link
Copy Markdown
Contributor

@StevenLuMT - what is the justification for removing this method? We added it as part of PIP 180. I think we should consult with @Jason918 before we do this kind of cleanup up, as this feature was only just added in November with #18265 and might not be something we should remove.

@michaeljmarshall Thank you for the reminding. It's OK to delete this. I add this method during locally updates. It's not used any more.

@StevenLuMT
Copy link
Copy Markdown
Member Author

@StevenLuMT - what is the justification for removing this method? We added it as part of PIP 180. I think we should consult with @Jason918 before we do this kind of cleanup up, as this feature was only just added in November with #18265 and might not be something we should remove.

@michaeljmarshall Thank you for the reminding. It's OK to delete this. I add this method during locally updates. It's not used any more.

@michaeljmarshall I have communicated with @Jason918 , it's not use any more,
thank you very much

@michaeljmarshall
Copy link
Copy Markdown
Member

Great! Thanks for verifying.

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

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants