namest504 commented on PR #24809: URL: https://github.com/apache/pulsar/pull/24809#issuecomment-4931177445
@lhotari I've addressed the review comments and resolved the conflicts with master (rebuilt the refactoring on top of the current Gradle/slog codebase). Main changes since your last review: - Kept the original `TableViewImpl` class name and moved the shared logic to a package-private `AbstractTableViewImpl<T, V>`, following the naming in your experiment branch. No imports are removed and there are no fully qualified class name references anymore. - Removed the unused `MessageTableView` class left over from the earlier `createForMessages` approach. - Disabled message pooling for mapped views: the mapper may retain the `Message` instance, so the pooled payload buffer must not be reused or released. The classic `TableView` path keeps pooling and releasing as before. - Added mapper null-validation and unit tests in `TableViewBuilderImplTest`. The vote result for PIP-445 has been posted on the dev mailing list (3 binding +1s), and I've also updated the PIP document in #24842 to match the final class names. PTAL when you have a chance. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
