Dear Nikhil, I would like to make an emphasis on the following
Critical Changes: 1) Remove hyphens from child account numbers: I have explained in depth why we should do this in my feedback. Grouping Clients is paramount especially in East and West Africa and that is how i found the name(Abreviation GLIM/GSIM). Group Lending with Individual Monitoring / Group Savings with Individual Monitoring The GLIM - GSIM has helped a lot in getting microfinance companies to use MifosX One major challenge i was faced with is this Module is the hyphen- Clients in a a group that have GSIM accounts with hyphens ; 1. can not be issued with check-books 2. can not have BBANs generated "comfortably" as BBANs have a standard format that does not allow for hyphens 3. the Maximum Account number should 10 digits on with other numbers are added using a standard algorithm used for BBANs With hyphens in place, this limits the intention of MifosX users to provide extra financial services to their clients. I need to be pointed to the repo so that i can make detailed comments of my extensive reviews and tests on the GLIM / GSIM My Kind Regards Francis Guchie Kirago *Skype:* francisguchie *Whatsapp: *232 79 19 44 07 *LINKEDIN:* https://www.linkedin.com/in/francis-guchie-kirago-a4379617/ twitter: @FrancisGuchie On Sun, Apr 19, 2020 at 9:04 PM Nikhil Pawar <[email protected]> wrote: > Thank you Francis- your inputs would be greatly appreciated. > > Rahul > > I went through the code and have already commented my feedback on the > files. > Here is the list for your reference according to the priority: > > Critical Changes: > 1) Remove hyphens from child account numbers: I have explained in depth > why we should do this in my feedback. > 2) Revert persistence.xml change as suggested by Michael. > 3) Add concurrency fix in GSIM. > > Non-critical changes: > 4) Align GLIM account creation mechanism with GSIM( if possible). > 5) Avoid appending sql params in sql string and instead pass them as > object params. > 6) Omit use of glimID and gsimID from commandProcessing result > 7)Remove unwanted whitespaces and new lines in PR. > 8) extract functions out of repetitive code in > LoanApplicationWritePlatformServiceJpaRepositoryImpl > and SavingsAccountWritePlatformServiceJpaRepositoryImpl > > > I would request you to address above stuff based on the priority. This PR > has a go ahead from me, if critical changes as addressed. > > If you need help, please feel to ping me and I would be happy to help you. > > Regards, > Nikhil > > > > > On Sat, Apr 18, 2020 at 1:42 PM Francis Guchie <[email protected]> > wrote: > >> Yes Nikhil, >> >> I will review >> >> My Kind Regards >> >> Francis Guchie Kirago >> *Skype:* francisguchie >> *Whatsapp: *232 79 19 44 07 >> *LINKEDIN:* https://www.linkedin.com/in/francis-guchie-kirago-a4379617/ >> twitter: @FrancisGuchie >> >> >> >> >> On Sat, Apr 18, 2020 at 5:12 PM Nikhil Pawar <[email protected]> wrote: >> >>> ++ Francis >>> If you have time could you please help in doing functional review. >>> >>> I will also go through the code and provide my feedback. But I would >>> strongly recommend that we merge this PR , provided it does not break any >>> existing functionality. Rebasing this huge chunk of code takes lot of >>> effort and now that we have a rebased code - this opportunity should not be >>> missed. >>> >>> Besides that, this feature also has front end support. The next task to >>> be picked up would be to rebase front end code. Once we have front end >>> support- it would be much easier for functional reviewers to test this >>> feature. >>> >>> Regards, >>> Nikhil >>> >>> On Sat, Apr 18, 2020 at 12:35 PM Michael Vorburger <[email protected]> >>> wrote: >>> >>>> Community, >>>> >>>> Could parties interested in >>>> https://issues.apache.org/jira/browse/FINERACT-603 please help with >>>> code reviewing https://github.com/apache/fineract/pull/738 ? >>>> >>>> If nobody cares enough to spend time to help doing reviews in the next >>>> say 2 weeks, then I propose we'll just merge it.... >>>> >>>> Tx! >>>> M. >>>> _______________________ >>>> Michael Vorburger >>>> http://www.vorburger.ch >>>> >>>> >>>> On Mon, Mar 23, 2020 at 2:24 AM Michael Vorburger <[email protected]> >>>> wrote: >>>> >>>>> On Fri, Mar 20, 2020 at 5:28 AM RAHUL PAWAR <[email protected]> >>>>> wrote: >>>>> >>>>>> Hi Ed, >>>>>> >>>>>> Yes, the build is passing locally in machine. >>>>>> >>>>> >>>>> That is very curious, I don't see how that's possible; you should see >>>>> the same failures that Travis CI reports... >>>>> >>>>> ...do make sure that you rebase to the latest master, and run a full >>>>> "./gradlew build" and you will reproduce the Checkstyle errors that Travis >>>>> shows if you click on Details. >>>>> >>>>> I've also just commented on the >>>>> https://github.com/apache/fineract/pull/738 with some more >>>>> suggestions re. Checkstyle, for anyone interested to read. >>>>> >>>>> >>>>>> Thanks. >>>>>> >>>>>> On Fri, Mar 20, 2020 at 6:37 AM Ed Cable <[email protected]> wrote: >>>>>> >>>>>>> Wonderful news. We'll try to get it merged promptly. >>>>>>> >>>>>>> I'm adding @Dev <[email protected]> >>>>>>> >>>>>>> It's showing the Travis CI build is failing - the build is passing >>>>>>> locally for you? >>>>>>> >>>>>>> On Thu, Mar 19, 2020 at 7:04 AM RAHUL PAWAR <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi Ed, >>>>>>>> >>>>>>>> The Integration Test for glim_gsim has been completed and along >>>>>>>> with it, also fixed some bugs regarding it. >>>>>>>> >>>>>>>> Pr: https://github.com/apache/fineract/pull/738 >>>>>>>> >>>>>>>> Thankyou. >>>>>>>> >>>>>>>> On Thu, Jan 2, 2020 at 8:22 AM Ed Cable <[email protected]> wrote: >>>>>>>> >>>>>>>>> Thank you for the update. >>>>>>>>> >>>>>>>>> On Wed, Jan 1, 2020, 07:02 RAHUL PAWAR <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi Ed, >>>>>>>>>> I am still working on testcases, will be completed soon. >>>>>>>>>> >>>>>>>>>> On Wed, Nov 13, 2019 at 7:23 AM Nikhil Pawar <[email protected]> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> I am going to take this up with Rahul this holidays, will keep >>>>>>>>>>> you posted. >>>>>>>>>>> >>>>>>>>>>> On Tue, Nov 12, 2019 at 8:45 PM Ed Cable <[email protected]> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Rahul and Nikhil, >>>>>>>>>>>> >>>>>>>>>>>> I would love to get this work shipped to the community. Did you >>>>>>>>>>>> get a chance to write the necessary test cases? It's likely we'll >>>>>>>>>>>> target >>>>>>>>>>>> 1.5 release for this. >>>>>>>>>>>> >>>>>>>>>>>> See https://github.com/apache/fineract/pull/527 >>>>>>>>>>>> >>>>>>>>>>>> and https://issues.apache.org/jira/browse/FINERACT-603 >>>>>>>>>>>> >>>>>>>>>>>> Ed >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *Ed Cable* >>>>>>> President/CEO, Mifos Initiative >>>>>>> [email protected] | Skype: edcable | Mobile: +1.484.477.8649 >>>>>>> >>>>>>> *Collectively Creating a World of 3 Billion Maries | * >>>>>>> http://mifos.org <http://facebook.com/mifos> >>>>>>> <http://www.twitter.com/mifos> >>>>>>> >>>>>>>
