Thanks Jacques,
Sounds good. I will take care of this suggestion in coming commits.
Thanks & Regards,
Harsh
On Wednesday 31 August 2016 04:51 PM, Jacques Le Roux wrote:
OK I checked, the commented out lines were from pre Apache Era. So
indeed it was not an easy decision.
For
public void print(List<BOMNode> arr, BigDecimal quantity, int
depth, boolean excludeWIPs) {
I believe the lines were commented out because it's a recursive
method. I still believe we should never let exceptions escape. The
probability it happens is low. Another reason to not let it escape: it
should not clutter the log but when really needed.
So I simply suggest to add
Debug.logError(e, "Problem calling the " + serviceName + " service
(called by the createManufacturingOrder service)", module);
there.
Globally here is my take
Index:
applications/manufacturing/src/main/java/org/apache/ofbiz/manufacturing/bom/BOMNode.java
===================================================================
---
applications/manufacturing/src/main/java/org/apache/ofbiz/manufacturing/bom/BOMNode.java
(revision 1758522)
+++
applications/manufacturing/src/main/java/org/apache/ofbiz/manufacturing/bom/BOMNode.java
(working copy)
@@ -292,7 +292,7 @@
variantProduct =
variantProducts.get(0);
}
} catch (GenericServiceException e) {
- if (Debug.infoOn())
Debug.logInfo("Error calling getProductVariant service " +
e.getMessage(), module);
+ Debug.logError("Error calling
getProductVariant service " + e.getMessage(), module);
}
if (variantProduct != null) {
newNode = new
BOMNode(variantProduct, dispatcher, userLogin);
@@ -433,7 +433,7 @@
this.quantity = calcQuantity;
}
} catch (GenericServiceException e) {
-
+ Debug.logError(e, "Problem calling the " +
serviceName + " service (called by the createManufacturingOrder
service)", module);
}
} else {
this.quantity =
quantity.multiply(quantityMultiplier).multiply(scrapFactor);
@@ -573,7 +573,7 @@
}
}
} catch (GenericEntityException e) {
-
+ Debug.logError(e, "Problem calling the
getManufacturingComponents service", module);
}
}
return UtilMisc.toMap("productionRunId", productionRunId,
"endDate", endDate);
What to you think?
Jacques
Le 30/08/2016 à 11:21, Jacques Le Roux a écrit :
Le 30/08/2016 à 08:29, Jacopo Cappellato a écrit :
Highlighting code that could be improved rather than fixing it is a
good
way to help potential contributors.
However, and I think this is the reason for Scott's remark, you
should not
have addressed your review/request to individual
committer/contributor (if
the defect you have noticed was not introduced by their
contribution, as in
this case).
OK, got the subtle nuance, thanks
Jacques
