[ 
https://issues.apache.org/jira/browse/ARTEMIS-3113?focusedWorklogId=550964&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-550964
 ]

ASF GitHub Bot logged work on ARTEMIS-3113:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 10/Feb/21 20:50
            Start Date: 10/Feb/21 20:50
    Worklog Time Spent: 10m 
      Work Description: clebertsuconic commented on a change in pull request 
#3448:
URL: https://github.com/apache/activemq-artemis/pull/3448#discussion_r574066419



##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/CoreAmqpConverter.java
##########
@@ -171,15 +165,15 @@ public static AMQPMessage fromCore(ICoreMessage 
coreMessage, StorageManager stor
             properties.setMessageId("ID:" + 
message.getInnerMessage().getUserID().toString());
          }
       }
-      Destination destination = message.getJMSDestination();
+      SimpleString destination = message.getJMSDestination();
       if (destination != null) {
-         properties.setTo(toAddress(destination));
-         maMap.put(JMS_DEST_TYPE_MSG_ANNOTATION, 
AMQPMessageSupport.destinationType(destination));
+         properties.setTo(destination.toString());

Review comment:
       @michaelandrepearce I did not want to depend on JMS at the first time I 
implemented the converter. I did it back then because it was the easy route. 
(copy and paste the client API, depend on the interfaces and get done with it).
   
   Now I need to remove the dependency. Using concrete classes is the way to go 
here.. I don't think there's much to discuss. .unless everybody is opposing to 
this change.. which I can't see it as a bad thing.




----------------------------------------------------------------
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]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 550964)
    Time Spent: 7.5h  (was: 7h 20m)

> Artemis AMQP shouldn't depend on JMS
> ------------------------------------
>
>                 Key: ARTEMIS-3113
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-3113
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>          Components: AMQP
>    Affects Versions: 2.16.0
>            Reporter: Emmanuel Hugonnet
>            Priority: Major
>          Time Spent: 7.5h
>  Remaining Estimate: 0h
>
> The converters between core messages and amqp messages depend on JMS. Since 
> this is just for convenience we should remove this dependecy on an API that 
> is being replaced.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to