Re: svn commit: r571591 - /ofbiz/branches/release4.0/app
Locked
Re: svn commit: r571591 - /ofbiz/branches/release4.0/app
 
|
Jacques, I've commented on this a lot before, but this REALLY isn't a bug fix, at all. This is definitely in the improvement camp, and just an improvement in error messages and handling. While maintaining the release branch is really great, just remember the rule for releases is: LESS IS MORE! Every change to the release branch creates the possibility of introducing MORE bugs. So if it's not a bug fix, we don't want to do it. Just so we're all on the same page: what we REALLY need for the release branch is use and testing. Just applying fixes from the trunk to the branch will never get it to a releasable state... -David [hidden email] wrote: > Author: jleroux > Date: Fri Aug 31 14:22:44 2007 > New Revision: 571591 > > URL: http://svn.apache.org/viewvc?rev=571591&view=rev > Log: > Applied fix from trunk for revision: 567520 > > Modified: > ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java > > Modified: ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java > URL: http://svn.apache.org/viewvc/ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java?rev=571591&r1=571590&r2=571591&view=diff > ============================================================================== > --- ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java (original) > +++ ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java Fri Aug 31 14:22:44 2007 > @@ -108,7 +108,7 @@ > } > } > if (this.orderHeader == null) { > - throw new IllegalArgumentException("Order header is not valid"); > + throw new IllegalArgumentException("Order header passed in is not valid for orderId [" + orderHeader.getString("orderId") + "]"); > } > } > > @@ -125,7 +125,12 @@ > try { > this.orderHeader = delegator.findByPrimaryKey("OrderHeader", UtilMisc.toMap("orderId", orderId)); > } catch (GenericEntityException e) { > - throw new IllegalArgumentException("Invalid orderId"); > + String errMsg = "Error finding order with ID [" + orderId + "]: " + e.toString(); > + Debug.logError(e, errMsg, module); > + throw new IllegalArgumentException(errMsg); > + } > + if (this.orderHeader == null) { > + throw new IllegalArgumentException("Order not found with orderId [" + orderId + "]"); > } > } > > @@ -155,7 +160,7 @@ > GenericValue productStore = delegator.findByPrimaryKeyCache("ProductStore", UtilMisc.toMap("productStoreId", productStoreId)); > return productStore; > } catch (GenericEntityException ex) { > - Debug.logError("Failed to get product store for order header [" + orderHeader + "] due to exception "+ ex.getMessage(), module); > + Debug.logError(ex, "Failed to get product store for order header [" + orderHeader + "] due to exception "+ ex.getMessage(), module); > return null; > } > } > > |
Re: svn commit: r571591 - /ofbiz/branches/release4.0/app
Administrator
|
David,
De : "David E Jones" <[hidden email]> > > Jacques, > > I've commented on this a lot before, but this REALLY isn't a bug fix, at all. This is definitely in the improvement camp, and just an improvement in error messages and handling. OK, I reverted from previous file version, sorry for misunderstanding. Actually this relieves me from worries I had when commiting it in 1st place. Actually I was trying to commit Hans changes in rev. 571504 ("correction for an error when editing a orderitem, reported by sumit and tested by sumit and Bilgin") and because I got conflicts I thought this changes in trunk were needed (were the last previous). But I was already suspecting that it was not a good idea because I was not sure both fixed bugs. Though "correction for an error" suggest this I had vaguely followed the thread which ended by Hans's fix, and then was not sure what it was really intended for (hope you get the idea at that point, sorry for confusion) > While maintaining the release branch is really great, just remember the rule for releases is: LESS IS MORE! > > Every change to the release branch creates the possibility of introducing MORE bugs. So if it's not a bug fix, we don't want to do it. > > Just so we're all on the same page: what we REALLY need for the release branch is use and testing. Just applying fixes from the trunk to the branch will never get it to a releasable state... Yes sure, I agree, all is a matter of free time... Thanks for advices Jacques > > -David > > > [hidden email] wrote: > > Author: jleroux > > Date: Fri Aug 31 14:22:44 2007 > > New Revision: 571591 > > > > URL: http://svn.apache.org/viewvc?rev=571591&view=rev > > Log: > > Applied fix from trunk for revision: 567520 > > > > Modified: > > ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java > > > > Modified: ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java > > URL: > > ============================================================================== > > --- ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java (original) > > +++ ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java Fri Aug 31 14:22:44 2007 > > @@ -108,7 +108,7 @@ > > } > > } > > if (this.orderHeader == null) { > > - throw new IllegalArgumentException("Order header is not valid"); > > + throw new IllegalArgumentException("Order header passed in is not valid for orderId [" + orderHeader.getString("orderId") + "]"); > > } > > } > > > > @@ -125,7 +125,12 @@ > > try { > > this.orderHeader = delegator.findByPrimaryKey("OrderHeader", UtilMisc.toMap("orderId", orderId)); > > } catch (GenericEntityException e) { > > - throw new IllegalArgumentException("Invalid orderId"); > > + String errMsg = "Error finding order with ID [" + orderId + "]: " + e.toString(); > > + Debug.logError(e, errMsg, module); > > + throw new IllegalArgumentException(errMsg); > > + } > > + if (this.orderHeader == null) { > > + throw new IllegalArgumentException("Order not found with orderId [" + orderId + "]"); > > } > > } > > > > @@ -155,7 +160,7 @@ > > GenericValue productStore = delegator.findByPrimaryKeyCache("ProductStore", UtilMisc.toMap("productStoreId", > > return productStore; > > } catch (GenericEntityException ex) { > > - Debug.logError("Failed to get product store for order header [" + orderHeader + "] due to exception "+ ex.getMessage(), module); > > + Debug.logError(ex, "Failed to get product store for order header [" + orderHeader + "] due to exception "+ ex.getMessage(), module); > > return null; > > } > > } > > > > |
Re: svn commit: r571591 - /ofbiz/branches/release4.0/app
|
|
Thanks for your understanding Jacques. Like you said, this is just a matter of time people can contribute to the project, and it's great that you are doing what you can! -David Jacques Le Roux wrote: > David, > > De : "David E Jones" <[hidden email]> >> Jacques, >> >> I've commented on this a lot before, but this REALLY isn't a bug fix, at all. This is definitely in the improvement camp, and just > an improvement in error messages and handling. > > OK, I reverted from previous file version, sorry for misunderstanding. Actually this relieves me from worries I had when commiting > it in 1st place. Actually I was trying to commit Hans changes in rev. 571504 ("correction for an error when editing a orderitem, > reported by sumit and tested by sumit and Bilgin") and because I got conflicts I thought this changes in trunk were needed (were the > last previous). But I was already suspecting that it was not a good idea because I was not sure both fixed bugs. Though "correction > for an error" suggest this I had vaguely followed the thread which ended by Hans's fix, and then was not sure what it was really > intended for (hope you get the idea at that point, sorry for confusion) > >> While maintaining the release branch is really great, just remember the rule for releases is: LESS IS MORE! >> >> Every change to the release branch creates the possibility of introducing MORE bugs. So if it's not a bug fix, we don't want to do > it. >> Just so we're all on the same page: what we REALLY need for the release branch is use and testing. Just applying fixes from the > trunk to the branch will never get it to a releasable state... > > Yes sure, I agree, all is a matter of free time... > > Thanks for advices > > Jacques > >> -David >> >> >> [hidden email] wrote: >>> Author: jleroux >>> Date: Fri Aug 31 14:22:44 2007 >>> New Revision: 571591 >>> >>> URL: http://svn.apache.org/viewvc?rev=571591&view=rev >>> Log: >>> Applied fix from trunk for revision: 567520 >>> >>> Modified: >>> ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java >>> >>> Modified: ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java >>> URL: > http://svn.apache.org/viewvc/ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java?rev=571591&r1=571590&r2=571591&view=diff >>> ============================================================================== >>> --- ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java (original) >>> +++ ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java Fri Aug 31 14:22:44 2007 >>> @@ -108,7 +108,7 @@ >>> } >>> } >>> if (this.orderHeader == null) { >>> - throw new IllegalArgumentException("Order header is not valid"); >>> + throw new IllegalArgumentException("Order header passed in is not valid for orderId [" + > orderHeader.getString("orderId") + "]"); >>> } >>> } >>> >>> @@ -125,7 +125,12 @@ >>> try { >>> this.orderHeader = delegator.findByPrimaryKey("OrderHeader", UtilMisc.toMap("orderId", orderId)); >>> } catch (GenericEntityException e) { >>> - throw new IllegalArgumentException("Invalid orderId"); >>> + String errMsg = "Error finding order with ID [" + orderId + "]: " + e.toString(); >>> + Debug.logError(e, errMsg, module); >>> + throw new IllegalArgumentException(errMsg); >>> + } >>> + if (this.orderHeader == null) { >>> + throw new IllegalArgumentException("Order not found with orderId [" + orderId + "]"); >>> } >>> } >>> >>> @@ -155,7 +160,7 @@ >>> GenericValue productStore = delegator.findByPrimaryKeyCache("ProductStore", UtilMisc.toMap("productStoreId", > productStoreId)); >>> return productStore; >>> } catch (GenericEntityException ex) { >>> - Debug.logError("Failed to get product store for order header [" + orderHeader + "] due to exception "+ > ex.getMessage(), module); >>> + Debug.logError(ex, "Failed to get product store for order header [" + orderHeader + "] due to exception "+ > ex.getMessage(), module); >>> return null; >>> } >>> } >>> >>> > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |