You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Harsh Vijaywargiya <ha...@hotwaxsystems.com> on 2016/09/01 13:28:22 UTC
Re: svn commit: r1757991 - in /ofbiz/trunk/applications/manufacturing/src/main/java/org/apache/ofbiz/manu
facturing: bom/ jobshopmgt/ mrp/ techdata/
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
>>
>>
>
Re: svn commit: r1757991 - in
/ofbiz/trunk/applications/manufacturing/src/main/java/org/apache/ofbiz/manu
facturing: bom/ jobshopmgt/ mrp/ techdata/
Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes thanks Pierre, just found it in svn annotations, then understood your answer :/
Jacques
Le 20/03/2017 � 11:22, Pierre Smits a �crit :
> Hey Jacques,
>
> This appears to be the related issue: OFBIZ-7848
>
> Best regards,
>
> Pierre Smits
>
> ORRTIZ.COM <http://www.orrtiz.com>
> OFBiz based solutions & services
>
> OFBiz Extensions Marketplace
> http://oem.ofbizci.net/oci-2/
>
> On Mon, Mar 20, 2017 at 11:19 AM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> What is the situation here, please? Is there a Jira? Should I take care of
>> that?
>>
>> Thanks
>>
>> Jacques
>>
>>
>>
>> Le 01/09/2016 � 15:28, Harsh Vijaywargiya a �crit :
>>
>>> 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(quantityMult
>>>> iplier).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
>>>>>
>>>>>
>>>>>
>>>
Re: svn commit: r1757991 - in /ofbiz/trunk/applications/manufacturing/src/main/java/org/apache/ofbiz/manu
facturing: bom/ jobshopmgt/ mrp/ techdata/
Posted by Pierre Smits <pi...@gmail.com>.
Hey Jacques,
This appears to be the related issue: OFBIZ-7848
Best regards,
Pierre Smits
ORRTIZ.COM <http://www.orrtiz.com>
OFBiz based solutions & services
OFBiz Extensions Marketplace
http://oem.ofbizci.net/oci-2/
On Mon, Mar 20, 2017 at 11:19 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> What is the situation here, please? Is there a Jira? Should I take care of
> that?
>
> Thanks
>
> Jacques
>
>
>
> Le 01/09/2016 à 15:28, Harsh Vijaywargiya a écrit :
>
>> 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(quantityMult
>>> iplier).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
>>>>
>>>>
>>>>
>>>
>>
>>
>
Re: svn commit: r1757991 - in
/ofbiz/trunk/applications/manufacturing/src/main/java/org/apache/ofbiz/manu
facturing: bom/ jobshopmgt/ mrp/ techdata/
Posted by Jacques Le Roux <ja...@les7arts.com>.
What is the situation here, please? Is there a Jira? Should I take care of that?
Thanks
Jacques
Le 01/09/2016 � 15:28, Harsh Vijaywargiya a �crit :
> 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
>>>
>>>
>>
>
>