Perhaps your argument against the final keyword is that it should never be used? If so, you and @wido can debate that and let me know which way you’d like this bit of code to end up.
> On Jan 20, 2018, at 9:05 AM, Tutkowski, Mike <mike.tutkow...@netapp.com> > wrote: > > It’s pretty much the reason why the final variable exists in Java: to make a > variable a constant so no one accidentally changes it. @wido just wanted the > code to protect against the variable being changed...that’s all. > >> On Jan 20, 2018, at 8:25 AM, Rafael Weingärtner >> <rafaelweingart...@gmail.com> wrote: >> >> Changed by a future programmer that may develop some changes in your code? >> Because, if you do not code any other assignment to that variable, it will >> never happen. >> >> On Sat, Jan 20, 2018 at 1:16 PM, Tutkowski, Mike <mike.tutkow...@netapp.com> >> wrote: >> >>> @wido just wanted to make sure the Boolean variable didn’t accidentally >>> get changed later since it’s critical it keep that value. >>> >>>> On Jan 20, 2018, at 8:14 AM, Tutkowski, Mike <mike.tutkow...@netapp.com> >>> wrote: >>>> >>>> I made it final per @wido’s comment (in #2415). How’s about you guys >>> hash it out and get back to me. :) >>>> >>>>> On Jan 20, 2018, at 8:06 AM, GitBox <g...@apache.org> wrote: >>>>> >>>>> rafaelweingartner commented on a change in pull request #2416: >>> CLOUDSTACK-10244: KVM online storage migration fails >>>>> URL: https://github.com/apache/cloudstack/pull/2416# >>> discussion_r162784630 >>>>> >>>>> >>>>> >>>>> ########## >>>>> File path: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/ >>> resource/wrapper/LibvirtMigrateCommandWrapper.java >>>>> ########## >>>>> @@ -132,8 +132,9 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to >>> v1.0.0. >>>>> vmsnapshots = libvirtComputingResource. >>> cleanVMSnapshotMetadata(dm); >>>>> >>>>> Map<String, MigrateCommand.MigrateDiskInfo> >>> mapMigrateStorage = command.getMigrateStorage(); >>>>> + final boolean migrateStorage = MapUtils.isNotEmpty( >>> mapMigrateStorage); >>>>> >>>>> Review comment: >>>>> This variable does not need to be final. It does not bring any >>> benefits here. >>>>> >>>>> You needed to assign the result of the first >>>>> `MapUtils.isNotEmpty(mapMigrateStorage)` >>> because the method `replaceStorage` might change the status of >>> `mapMigrateStorage`, right? >>>>> >>>>> ---------------------------------------------------------------- >>>>> 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: >>>>> us...@infra.apache.org >>>>> >>>>> >>>>> With regards, >>>>> Apache Git Services >>> >> >> >> >> -- >> Rafael Weingärtner
