Skip to content

Conversation

@HQebupt
Copy link
Contributor

@HQebupt HQebupt commented Jan 29, 2022

Master Issue: #14013

Motivation

See #14013

Verifying this change

Make sure that the change passes the CI checks.

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 29, 2022
@HQebupt HQebupt force-pushed the issue-14013-internalPeekNthMessage branch from 2410170 to 768ce72 Compare January 29, 2022 08:50
@HQebupt
Copy link
Contributor Author

HQebupt commented Jan 29, 2022

Thank you @codelipenghui PLAT

@HQebupt HQebupt force-pushed the issue-14013-internalPeekNthMessage branch from 759504d to abec3f1 Compare January 29, 2022 10:28
codelipenghui
codelipenghui previously approved these changes Jan 29, 2022
@HQebupt
Copy link
Contributor Author

HQebupt commented Jan 29, 2022

/pulsarbot rerun-failure-checks

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.

Another, I have two suggestions:

  • Remove asyncResponse from the internal method, and unified exception and result handling are performed at the controller layer, which makes the internal method purer.
  • Have you checked to see if there is a test for the modified method?

@HQebupt
Copy link
Contributor Author

HQebupt commented Jan 30, 2022

/pulsarbot rerun-failure-checks

1 similar comment
@HQebupt
Copy link
Contributor Author

HQebupt commented Jan 30, 2022

/pulsarbot rerun-failure-checks

@HQebupt
Copy link
Contributor Author

HQebupt commented Mar 6, 2022

/pulsarbot rerun-failure-checks

@HQebupt
Copy link
Contributor Author

HQebupt commented Apr 2, 2022

/pulsarbot rerun-failure-checks

@github-actions
Copy link

github-actions bot commented May 3, 2022

The pr had no activity for 30 days, mark with Stale label.

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

The pr had no activity for 30 days, mark with Stale label.

@HQebupt HQebupt closed this Jun 23, 2022
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 lifecycle/stale Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants