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
