Comments from Keith Wall while reviewing
QPID-3167, raised as a separate issue for resolution:
No comments against your actual changes but:
1) I think we should take this opportunity to add some class javadoc to the newly extracted QueueRunner.
2) In QueueRunner, _logger.error(e) is going to fail to log the underlying(s) in the broker log. This can make problem determination harder. Would be better to use the two argument form error(message,throwable)
3) I notice SubFlushRunner made its instance variables explicitly final. I think QueueRunner ought to have done the same. I think the stricter form is preferable.
I realise all of the above would have applied to code base prior to your change.
|Field||Original Value||New Value|
|Status||Open [ 1 ]||In Progress [ 3 ]|
|Status||In Progress [ 3 ]||Ready To Review [ 10006 ]|
|Assignee||Robbie Gemmell [ gemmellr ]||Keith Wall [ k-wall ]|
|Status||Ready To Review [ 10006 ]||Resolved [ 5 ]|
|Resolution||Fixed [ 1 ]|