You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "StevenLuMT (via GitHub)" <gi...@apache.org> on 2023/02/14 09:13:52 UTC

[GitHub] [pulsar] StevenLuMT opened a new pull request, #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

StevenLuMT opened a new pull request, #19513:
URL: https://github.com/apache/pulsar/pull/19513

   ### 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:
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   *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` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   PR in forked repository: https://github.com/StevenLuMT/pulsar/pull/5


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] StevenLuMT commented on pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on PR #19513:
URL: https://github.com/apache/pulsar/pull/19513#issuecomment-1430866204

   /pulsarbot run-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] StevenLuMT commented on pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on PR #19513:
URL: https://github.com/apache/pulsar/pull/19513#issuecomment-1434014775

   > > @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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] AnonHxy merged pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy merged PR #19513:
URL: https://github.com/apache/pulsar/pull/19513


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] michaeljmarshall commented on pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #19513:
URL: https://github.com/apache/pulsar/pull/19513#issuecomment-1434058725

   Great! Thanks for verifying.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] michaeljmarshall commented on pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #19513:
URL: https://github.com/apache/pulsar/pull/19513#issuecomment-1432575238

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] Jason918 commented on pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "Jason918 (via GitHub)" <gi...@apache.org>.
Jason918 commented on PR #19513:
URL: https://github.com/apache/pulsar/pull/19513#issuecomment-1434008735

   > @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] StevenLuMT commented on pull request #19513: [cleanup][broker] Cleanup ManagedLedgerImpl's nouse method: isLedgersReadonly

Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on PR #19513:
URL: https://github.com/apache/pulsar/pull/19513#issuecomment-1430811489

   /pulsarbot run-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org