----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18568/#review35618 -----------------------------------------------------------
In general I believe it makes sense to return false in this case, since it is clear that the host was not fenced. At the same time the reviewers dislike havig formating (finals) in the patches, it might be easier to push it through the process if you could separate the cleanup from the fix. Also, is there a bug url that should be included? - Laszlo Hornyak On Feb. 27, 2014, 8:03 a.m., Wilder Rodrigues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18568/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 8:03 a.m.) > > > Review request for cloudstack, daan Hoogland and Hugo Trippaers. > > > Repository: cloudstack-git > > > Description > ------- > > Fix for Find Bugs findings on troubling issues: returning null when expected > is boolean; adding 6 unit tests and fix 1 in the KVMFencer; comparing objects > with == instead of equals() > > > Diffs > ----- > > plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java 28cba2b > > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java > 48ae3ea > plugins/hypervisors/xen/test/com/cloud/ha/XenServerFencerTest.java bd1d8f8 > server/src/com/cloud/ha/KVMFencer.java 516a579 > server/test/com/cloud/ha/KVMFencerTest.java cdd13b6 > > Diff: https://reviews.apache.org/r/18568/diff/ > > > Testing > ------- > > Added 6 unit tests for XenServerFencer (removed an useless one which was > testing get/set methods) > Build completed successfully > > Tested also on DevCloud + XenServer the following: > > Create Zone + Network + Pod + cluster + Pri/Sec Storage + Console Proxy and > System VMS > Create 1 instance (tiny Linux) + Guest Network + 1 SourceNAT + 1 extra pub ip > Set up firewall for port 22 + ICMP + port forwarding 22 ==> instance > SSH into the instance > > > Thanks, > > Wilder Rodrigues > >
