Sometimes it's not easy to revert the commit, I suppose a bug should be raised and assigned to the individual, in that case. For example, someone branches and then merges without pulling in fixes that occurred in between, this is fairly common and can be painful when file locations are shifted around (as seems to happen quite frequently between releases), since the old file with the fixes is simply removed and the new, stale file is left.
On Mon, Feb 24, 2014 at 5:27 PM, Alex Huang <alex.hu...@citrix.com> wrote: > I think we should just revert the commits on problems. It actually helps the > developers because it gives them a chance to redo it in the correct way and > hopefully, they gain more understanding of what needs to be done. At worst, > they will remember if penalized enough times. Sometimes, tough love is > necessary. > > --Alex > >> -----Original Message----- >> From: Trippie [mailto:trip...@gmail.com] On Behalf Of Hugo Trippaers >> Sent: Monday, February 24, 2014 8:15 AM >> To: <dev@cloudstack.apache.org> >> Subject: Yet another mail on code quality >> >> Guys, >> >> Please pay attention to the code you are committing. Today i fixed a number >> of issues that were introduced in recent code, these are bugs that could have >> been prevented from entering master by either testing or running the >> findbugs checks. One was committed directly, the other one through a >> reviewed patch. >> >> >> @@ -116,7 +116,7 @@ public class Upgrade430to440 implements DbUpgrade >> { >> if (networkRs.next()) { >> String guesttype = networkRs.getString(1); >> >> - if (guesttype == >> Network.GuestType.Shared.toString()) { >> + if >> (guesttype.equals(Network.GuestType.Shared.toString())) >> { >> pstmtUpdate = conn.prepareStatement("UPDATE >> `cloud`.`user_ip_address` SET account_id = ?, domain_id= ? WHERE >> public_ip_address = ?"); >> pstmtUpdate.setLong(1,vmAccountId); >> pstmtUpdate.setLong(2,vmDomainId); >> >> >> >> @@ -80,11 +80,11 @@ public class LibvirtStoragePoolXMLParser { >> String targetPath = getTagValue("path", target); >> >> String portValue = getAttrValue("host", "port", source); >> - if (portValue != "") >> + if (portValue != null && !portValue.isEmpty()) >> port = Integer.parseInt(portValue); >> >> return new >> LibvirtStoragePoolDef(LibvirtStoragePoolDef.poolType.valueOf(format.toUp >> perCase()), >> >> >> To help, i've configured the slowbuild to alert if the number of high >> priority >> findings from findbugs differs from the previous run. It will notify all >> developers that had changes during this period (slowbuild runs every 4 >> hours). >> >> Cheers, >> >> Hugo
