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
