DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202337269
##########
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##########
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
- List<SnapshotDetailsVO> snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
- for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+ final List<SnapshotDetailsVO> snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
Actually it is kind of kidding yourself in this case, as the object ref may
not be changed after setting but the state can still be changed. I would advice
using a final filter against this as a spam filter against spam; ignore, don't
read.
In this case it is best to follow @rafaelweingartner 's suggestion, to make
the new and/or changed code be in smaller methods and leave the existing alone.
Though refactoring the whole method is an option as well of course ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
