Skip to content

Conversation

@315157973
Copy link
Contributor

Motivation

It is difficult to get CR due to so many modifications in PIP 81 #10729.
So I split this PR into multiple sub-PRs to facilitate CR

Modifications

I split the logic in createNewMetadataLedger into multiple methods to facilitate subsequent reuse

  1. Put the logic of creating Ledger into doCreateNewMetadataLedger separately
  2. switchToNewLedger does not need to wrap another layer of callback, so remove redundant wrapping
  3. When persisting data fails, we need to delete the created Ledger, this part of the logic is put into deleteLedger

Verifying this change

This modification does not change any previous behavior, so no unit tests are required, but the previous unit tests must be ensured to pass

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • no-need-doc
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-added
    (Docs have been already added)

@315157973 315157973 self-assigned this May 4, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 4, 2022
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mattisonchao mattisonchao left a comment

Choose a reason for hiding this comment

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

LGTM, left a trivial comment.

@315157973
Copy link
Contributor Author

/pulsarbot run-failure-checks

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants