You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ofbiz.apache.org by "danwatford (via GitHub)" <gi...@apache.org> on 2023/04/09 10:03:08 UTC

[GitHub] [ofbiz-framework] danwatford commented on pull request #618: Codenarc integration rebased

danwatford commented on PR #618:
URL: https://github.com/apache/ofbiz-framework/pull/618#issuecomment-1501092052

   Code review passed. 
   
   Review approach was to:
   - Compare the list of commits from the previous PR  (#517), ensuring they matched in terms of ordering.
   - Review the additional commits in this PR.
   
   Outcome of code review was a pass.
   
   Branch was successfully built and integration tests run.
   
   A commit highlighted some testing of line 215 from BalanceSheet.groovy was required. This code relates to how previous balances were included in BalanceSheet calculations.
   
   BalanceSheet.groovy was tested by running closing time periods and running reports for 2009-02-01, 2009-03-01, 2009-04-01 and comparing the results to demo-next.   Reports matched and test considered a PASS.
   
   Assuming no other reviewers highlight any issues, then I believe this PR can be merged to trunk. I am happy for others to do this or I will squash and merge via GitHub in a day or two.
   
   My only concern is that authorship of the commit is correctly attributed to @gilPts . I believe that squashing and merging via GitHub will correctly handle this, but someone please let me know if this is not the case.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ofbiz.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org