[GitHub] [geode] dschneider-pivotal commented on a change in pull request #6892: GEODE-9625: Only serialize transaction metadata when grouping enabled.

[GitBox <gi...@apache.org>]

dschneider-pivotal commented on a change in pull request #6892:
URL: https://github.com/apache/geode/pull/6892#discussion_r715750905



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySenderEventProcessor.java
##########
@@ -1225,6 +1228,13 @@ public void closeProcessor() {
     }
   }
 
+  protected GatewaySenderEventImpl.TransactionMetadataDisposition getTransactionMetadataDisposition(
+      final boolean isLastEventInTransaction) {
+    return getSender().mustGroupTransactionEvents()

Review comment:
       I think this would be easier to understand if you changed it to use if statements. 

##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java
##########
@@ -1354,4 +1355,20 @@ protected GatewaySenderEventImpl makeCopy() {
   public void setAcked(boolean acked) {
     isAcked = acked;
   }
+
+  public enum TransactionMetadataDisposition {
+    /**
+     * Transaction metadata should be excluded from the event.
+     */
+    Exclude,

Review comment:
       I think the geode style guide says constants should be all upper case. The members of an enum are constants. See https://google.github.io/styleguide/javaguide.html#s5.2-specific-identifier-names
   I think you should follow this convention for this new enum




-- 
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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org