BewareMyPower commented on pull request #8618: URL: https://github.com/apache/pulsar/pull/8618#issuecomment-731592732
See https://github.com/apache/pulsar/blob/0c71eb6da12db83b7f5a9def14316a6daf128349/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L1360-L1362 and https://github.com/apache/pulsar/blob/0c71eb6da12db83b7f5a9def14316a6daf128349/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java#L112-L114 These two code blocks both check `isBrokerTimestampForMessageEnable` and determine whether to serialize or deserialize RawMetadata. However, if we add a new field to `RawMessageMetadata`, e.g. `message_sequence_id`, the check needs to be changed to ```java if (config.isBrokerTimestampForMessageEnable() || config.isMessageSequenceIdEnable()) ``` Right? So we should wrap the check into a single method, like: ```java public boolean isRawMetadataEnable() { return isBrokerTimestampForMessageEnable() /* || isMessageSequenceIdEnable() or more 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. For queries about this service, please contact Infrastructure at: [email protected]
