michaelpearce-gain edited a comment on issue #2802: ARTEMIS-2457 implement ring queue URL: https://github.com/apache/activemq-artemis/pull/2802#issuecomment-524669540 Looking better. Nice seeing update and ability to enable an existing queue, and vice versa disable +1 for that. Was exactly what i meant in prev comment. left a number of comments (mostly nits wouldn't block merge), but... There is one critical bug i found, it was possible to raise NPE in QueueImpl constructor, this is because its not following the same pattern as all other fields, lets keep things uniform / follow existing convention, this way avoids issues such as this, and also people get used to certain area's expecting to do own certain responsibility when debugging etc. im happy if we want to change that convention and where address setting lookups occur when fields are null, but then we do it for all, for consistency - and ideally for that change to be done independently.
---------------------------------------------------------------- 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] With regards, Apache Git Services
