Skip to content

[pip] PIP-373 Support to specify message listeners in TableView constructor #23167

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 2 commits into
base: master
Choose a base branch
from

Conversation

heesung-sn
Copy link
Contributor

Please refer to this fork PR for an implementation example. heesung-sn#79

Copy link

@heesung-sn Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@Denovo1998
Copy link
Contributor

PIP-371 and PIP-372 already exist.

@heesung-sn heesung-sn changed the title [pip] PIP-371 Support to specify message listeners in TableView constructor [pip] PIP-373 Support to specify message listeners in TableView constructor Aug 14, 2024
pip/pip-369.md Outdated
Comment on lines 40 to 42
* If {@link TableViewBuilder#existingMessageListeners} are not specified,
* these listeners are used for both
* existing and tailing(future) messages in the topic.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that it's a great idea to use this for both types. If someone wants the listener on both types, why not just register it for the other type of listener separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will require setting the same listener for both, which I think is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont have strong opinion here, but I thought that we want the client design for less code.

@lhotari
Copy link
Member

lhotari commented Oct 9, 2024

@heesung-sn please up the mailing list threads to the document.

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

Successfully merging this pull request may close these issues.

3 participants