gemmellr commented on code in PR #5484: URL: https://github.com/apache/activemq-artemis/pull/5484#discussion_r1952645625
########## artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueFactoryImpl.java: ########## @@ -79,9 +79,9 @@ public void setPostOffice(final PostOffice postOffice) { public Queue createQueueWith(final QueueConfig config) { final Queue queue; if (lastValueKey(config) != null) { - queue = new LastValueQueue(config.id(), config.address(), config.name(), config.filter(), config.getPagingStore(), config.pageSubscription(), config.user(), config.isDurable(), config.isTemporary(), config.isAutoCreated(), config.deliveryMode(), config.maxConsumers(), config.isExclusive(), config.isGroupRebalance(), config.getGroupBuckets(), config.getGroupFirstKey(), config.consumersBeforeDispatch(), config.delayBeforeDispatch(), config.isPurgeOnNoConsumers(), lastValueKey(config), config.isNonDestructive(), config.isAutoDelete(), config.getAutoDeleteDelay(), config.getAutoDeleteMessageCount(), config.isConfigurationManaged(), scheduledExecutor, postOffice, storageManager, addressSettingsRepository, executorFactory.getExecutor(), server, this); + queue = new LastValueQueue(QueueConfiguration.of(config.name()).setAddress(config.address()).setId(config.id()).setUser(config.user()).setDurable(config.isDurable()).setTemporary(config.isTemporary()).setAutoCreated(config.isAutoCreated()).setRoutingType(config.deliveryMode()).setMaxConsumers(config.maxConsumers()).setExclusive(config.isExclusive()).setGroupRebalance(config.isGroupRebalance()).setGroupBuckets(config.getGroupBuckets()).setGroupFirstKey(config.getGroupFirstKey()).setConsumersBeforeDispatch(config.consumersBeforeDispatch()).setDelayBeforeDispatch(config.delayBeforeDispatch()).setPurgeOnNoConsumers(config.isPurgeOnNoConsumers()).setLastValueKey(lastValueKey(config)).setNonDestructive(config.isNonDestructive()).setAutoDelete(config.isAutoDelete()).setAutoDeleteDelay(config.getAutoDeleteDelay()).setAutoDeleteMessageCount(config.getAutoDeleteMessageCount()).setConfigurationManaged(config.isConfigurationManaged()), config.filter(), config.getPagingStore(), config.p ageSubscription(), scheduledExecutor, postOffice, storageManager, addressSettingsRepository, executorFactory.getExecutor(), server, this); Review Comment: I hadn't even noticed that the QueueConfig-using factory method I was commenting on _was itself also deprecated_ (since that doesnt show in the initial diff context of the change, and since the described aim was 'deprecated QueueImpl constructor removal', I simply didnt think to look further at the factory method since it was just being updated to use the new QueueImpl constructors). Given the factory methods were deprecated in the same commit as the QueueImpl constructors nearly 5 years ago, and you say the factories are also still really internal/impl detail, just removing the deprecated factory methods rather than updating them seems fair. -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact