cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Wilder Rodrigues" <wrodrig...@schubergphilis.com>
Subject Re: Review Request 18568: Fix for returning null when expected is boolean; comparing objects with == instead of equals()
Date Thu, 27 Feb 2014 13:20:13 GMT


> On Feb. 27, 2014, 12:10 p.m., Laszlo Hornyak wrote:
> > 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?

Hi Laszlo,

Concerning the "finals" it was due check-style combined with save actions in my Eclipse, just
to make sure the code is according to the CS templates. I understand it kind of make a bit
difficult to read the patch, but redoing it would require more effort than pushing it through.
We can agree that for the next I will do the formatting first, commit and send the patch.
Afterwards, I do the fix, squash the commit and update the patch. Is it a better idea?

Concerning the bug URL, there is none. I just picked up a package and ran find Bugs on it.


- Wilder


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18568/#review35618
-----------------------------------------------------------


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message