You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2009/10/02 18:49:37 UTC
Re: svn commit: r820846 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentWorker.java
lektran@apache.org wrote:
> Author: lektran
> Date: Thu Oct 1 23:38:38 2009
> New Revision: 820846
>
> URL: http://svn.apache.org/viewvc?rev=820846&view=rev
> Log:
> Removed two deprecated (>3 years) methods:
> getPartyPaymentMethodValueMaps(PageContext, String, Boolean, String)
> getPaymentMethodAndRelated(PageContext, String, String, String, String, String, String, String, String)
>
> Replaced a LinkedList with a FastList and a HashMap with a FastMap
> Added some generics markup
> A couple of enhanced for loops in place of iterator + while
> Made use of auto-boxing for a couple of booleans
This is an example of a bad checkin. It should have been done as 2
checkins. One, for the generics/java1.5/javolution changes. The
other checkin then would remove the methods.
The reason for this, is the method removal is an actual code
api/abi/feature change. Those types of commits should be separate, as
it makes debugging much easier. Just because those methods have been
removed *now*, doesn't mean external people using those methods will
notice. They will find out about it in 1-3 years, when they deploy a
new version of ofbiz, and suddenly their code stops working.
Re: svn commit: r820846 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentWorker.java
Posted by Scott Gray <sc...@hotwaxmedia.com>.
Thanks for your feedback Adam.
Regards
Scott
On 3/10/2009, at 5:49 AM, Adam Heath wrote:
> lektran@apache.org wrote:
>> Author: lektran
>> Date: Thu Oct 1 23:38:38 2009
>> New Revision: 820846
>>
>> URL: http://svn.apache.org/viewvc?rev=820846&view=rev
>> Log:
>> Removed two deprecated (>3 years) methods:
>> getPartyPaymentMethodValueMaps(PageContext, String, Boolean, String)
>> getPaymentMethodAndRelated(PageContext, String, String, String,
>> String, String, String, String, String)
>>
>> Replaced a LinkedList with a FastList and a HashMap with a FastMap
>> Added some generics markup
>> A couple of enhanced for loops in place of iterator + while
>> Made use of auto-boxing for a couple of booleans
>
> This is an example of a bad checkin. It should have been done as 2
> checkins. One, for the generics/java1.5/javolution changes. The
> other checkin then would remove the methods.
>
> The reason for this, is the method removal is an actual code
> api/abi/feature change. Those types of commits should be separate, as
> it makes debugging much easier. Just because those methods have been
> removed *now*, doesn't mean external people using those methods will
> notice. They will find out about it in 1-3 years, when they deploy a
> new version of ofbiz, and suddenly their code stops working.