fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] vorburger commented on issue #700: Fineract-820: Fix for LoanReschedulingWithinCenterTest fail on Sundays
Date Sun, 26 Jan 2020 21:50:45 GMT
vorburger commented on issue #700: Fineract-820:  Fix for LoanReschedulingWithinCenterTest
fail on Sundays
URL: https://github.com/apache/fineract/pull/700#issuecomment-578547028
 
 
   @angelboxes thanks a lot for raising this PR!  Given that [the develop branch failed to
build today on Sunday](https://travis-ci.org/apache/fineract/builds/641997405) (after the
requested revert of e9e0bbc930ded7e1b3be942d237e228bacdb52b8, because of that minor Checkstyle
problem), and given that this PR passed, it appears this fix is the correct solution (contrary
to the initial one which myself and @awasum tried on Jan 5/6th, see JIRA), and I'm therefore
in favour of  this ASAP.
   
   Before we click Merge, just one thing seems worth clarifying: Was this PR inspired by e9e0bbc930ded7e1b3be942d237e228bacdb52b8
from @nazeer1100126 ? (See https://lists.apache.org/thread.html/r853c0b8ad9387d9925f89da611bb31a93139153e2b364b552fca8d5e%40%3Cdev.fineract.apache.org%3E).

   
   If you did look at that code, it would be "correct" to attribute @nazeer1100126 as the
(original) author of this commit - would you like to do that? Just in cases you never did
anything like this before, FYI it's easy with git, you can just `git commit --amend --author="Shaik
Nazeer Hussain <nazeerhussain.shaik@gmail.com>"; git push --force`.  (Unless @nazeer1100126
states here that he doesn't care and is OK if we merge this with @angelboxes as that commit's
author.)
   
   If you came up with the same solution without having looked at that code, and independently
found the same solution, then you can just state that here, and it would seem fair to merge
this and attribute you instead.
   
   The only other and much smaller point would be if, after we merged this, you would be interested
to do a follow-up with the minor refactoring proposed in e9e0bbc930ded7e1b3be942d237e228bacdb52b8
to extract the same from ClientLoanIntegrationTest.java into some common place such as Utils.java.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message