On 5/04/2010, at 5:47 PM, Adam Heath wrote: > Scott Gray wrote: >> On 5/04/2010, at 5:11 PM, Adam Heath wrote: >> >>> [email protected] wrote: >>>> Author: doogie >>>> Date: Sun Apr 4 19:08:46 2010 >>>> New Revision: 930737 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=930737&view=rev >>>> Log: >>>> Make use of UtilObject.equalsHelper in storeAll. >>>> >>>> Modified: >>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java >>>> >>>> Modified: >>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=930737&r1=930736&r2=930737&view=diff >>>> ============================================================================== >>>> --- >>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java >>>> Sun Apr 4 19:08:46 2010 >>>> @@ -1450,7 +1450,7 @@ public class GenericDelegator implements >>>> if (value.containsKey(fieldName)) { >>>> Object fieldValue = value.get(fieldName); >>>> Object oldValue = existing.get(fieldName); >>>> - if ((fieldValue == null && oldValue != null) >>>> || (fieldValue != null && !fieldValue.equals(oldValue))) { >>>> + if (UtilObject.equalsHelper(oldValue, >>>> fieldValue)) { >>>> toStore.put(fieldName, fieldValue); >>>> atLeastOneField = true; >>>> } >>> Did anyone else review this commit? Or did you not understand the code? >>> >>> Yes, I screwed it up. But if no one reviews the commits, then how >>> will much bigger problems be found? This was a simple problem to >>> find, an obvious inverted test, yet no one noticed it. >>> >>> What's the point of having commits mailed to the list if no one will >>> review them? >> >> Try to avoid dumping 10-20 commits at a time and I'll try to avoid skimming >> over them. > > Dumping 20 commits at once, and not doing anything for a week, vs: > doing 2 a day, for 7 days, the same amount of review has to happen. > And, it's better doing them all at once, because you don't have to > constantly multi-task.
I guess it comes down to preference, I can spare a few minutes here and there but I can't spend 30 minutes reviewing 10 commits so I just cram it into the few minutes. Quite often all I end up doing is reading the commit message and not looking at the code at all unless it's something I'm interested in. > And, besides, this little commit was not part of some larger commit > flood anyways. I see 9 commits within half an hour from yesterday.
smime.p7s
Description: S/MIME cryptographic signature
