angelboxes edited a comment on issue #700: Fineract-820: Fix for LoanReschedulingWithinCenterTest fail on Sundays URL: https://github.com/apache/fineract/pull/700#issuecomment-578549495 > @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 [e9e0bbc](https://github.com/apache/fineract/commit/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 [e9e0bbc](https://github.com/apache/fineract/commit/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 <[email protected]>"; 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 [e9e0bbc](https://github.com/apache/fineract/commit/e9e0bbc930ded7e1b3be942d237e228bacdb52b8) to extract the same from ClientLoanIntegrationTest.java into some common place such as Utils.java. Hi, well actually this is not based on Nazeer commit and I did comment about this solution on Jiira on Tuesday, which at the time it was assigned to you, I didn't reassigned to myself for that reason and also I was waiting for today just to be sure that this would run on Travis. I got no response and saw that Nazeer assigned to himself some days after that and guessed that he would fix this issue. As you can see on Jiira comments I just did this pull request just to show that what I pointed Iniitially was the problem and that we shouldn't gave up so easily on this issue. I have no problem if this gets closed and Nazeer recommits his code.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
