Github user calohmn commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2256#discussion_r211574987
  
    --- Diff: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
 ---
    @@ -733,6 +743,9 @@ public void sendBuffer(ByteBuf buffer, int 
deliveryCount) {
              
buffer.writeBytes(data.duplicate().limit(headerEnds).byteBuffer());
           }
     
    +      writeOutgoingDeliveryAnnotations(buffer);
    --- End diff --
    
    I'm not sure here:
    This addition here (in `sendBuffer(buffer, deliveryCount)` from the 
`Message` interface) was done to have both the `sendBuffer` and `getSendBuffer` 
methods behave the same towards using the `outgoingDeliveryAnnotations`.
    
    But on the other hand this `sendBuffer` method isn't called by the 
`ProtonServerSenderContext` (only `getSendBuffer` is).
    So maybe the better solution would be to remove usage of 
`outgoingDeliveryAnnotations` in the `sendBuffer` method here and rename 
`getSendBuffer` to `getOutgoingMessageSendBuffer` to avoid confusion concerning 
these methods.


---

Reply via email to