priyasharma1 opened a new pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166 (OFBIZ-11754) - Since the MODULE variable could not be found, the service was throwing error, thus removed the variable - Instead used the groovy "logInfo" method directly to print the log, which has access to the module variable implicitly. - Removed the unused import, as Debug class is no longer needed here. Thanks: Pawan Verma for the guidance and discussion ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
verma-pawan commented on a change in pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166#discussion_r432636054 ########## File path: applications/content/groovyScripts/content/ContentServices.groovy ########## @@ -115,7 +113,7 @@ def createContentAlternativeUrl() { dataResourceId = serviceResult.dataResourceId } } catch (GenericServiceException e) { - Debug.logInfo(e, MODULE) Review comment: Hi @priyasharma1 As we are logging exception here, Shouldn't it be converted to logError()? WDYT? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
JacquesLeRoux commented on pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166#issuecomment-636300967 Also why not using logError (from GroovyBaseScript.groovy) instead of Debug.logError? I even wonder if we should change that everywhere. But then a Jira and maybe an agreement on dev ML would be required (later does not seem required to me) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
priyasharma1 commented on a change in pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166#discussion_r432824737 ########## File path: applications/content/groovyScripts/content/ContentServices.groovy ########## @@ -115,7 +113,7 @@ def createContentAlternativeUrl() { dataResourceId = serviceResult.dataResourceId } } catch (GenericServiceException e) { - Debug.logInfo(e, MODULE) Review comment: Thanks @verma-pawan I have the changes as per your suggestion. Earlier, since the original XML service used break-on-error="false" without any logs at all, so I could not decide whether to log error or info. So thank you for your inputs in clearing up my doubt. ########## File path: applications/content/groovyScripts/content/ContentServices.groovy ########## @@ -115,7 +113,7 @@ def createContentAlternativeUrl() { dataResourceId = serviceResult.dataResourceId } } catch (GenericServiceException e) { - Debug.logInfo(e, MODULE) Review comment: Thanks @verma-pawan I have made the changes as per your suggestion. Earlier, since the original XML service used break-on-error="false" without any logs at all, so I could not decide whether to log error or info. So thank you for your inputs in clearing up my doubt. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
priyasharma1 closed pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166 ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
priyasharma1 commented on pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166#issuecomment-636302627 > Also why not using logError (from GroovyBaseScript.groovy) instead of Debug.logError? > > I even wonder if we should change that everywhere. But then a Jira and maybe an agreement on dev ML would be required (later does not seem required to me) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
priyasharma1 edited a comment on pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166#issuecomment-636302627 > Also why not using logError (from GroovyBaseScript.groovy) instead of Debug.logError? > > I even wonder if we should change that everywhere. But then a Jira and maybe an agreement on dev ML would be required (later does not seem required to me) Yes, Jacques, I have used the same in the updated commit. Thanks! ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
JacquesLeRoux commented on pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166#issuecomment-636304170 Ha Pawan already created ttps://issues.apache.org/jira/browse/OFBIZ-11762 (" Use GroovyBaseScript's logging utility methods instead of using Debug in each Groovy files") :) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
In reply to this post by GitBox
JacquesLeRoux merged pull request #166: URL: https://github.com/apache/ofbiz-framework/pull/166 ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] |
Free forum by Nabble | Edit this page |