Thanks Michael, Francis and Nikhil for your feedback. I will address the concerns raised, based on priority. I will keep you guys posted on the progress.
On Mon, Apr 20, 2020 at 7:56 PM Francis Guchie <francisguc...@gmail.com> wrote: > 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 <npa...@apache.org> 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 <francisguc...@gmail.com> >> 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 <nickr...@gmail.com> 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 <m...@vorburger.ch> >>>> 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 <m...@vorburger.ch> >>>>> wrote: >>>>> >>>>>> On Fri, Mar 20, 2020 at 5:28 AM RAHUL PAWAR <rrpawar141...@gmail.com> >>>>>> 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 <edca...@mifos.org> wrote: >>>>>>> >>>>>>>> Wonderful news. We'll try to get it merged promptly. >>>>>>>> >>>>>>>> I'm adding @Dev <dev@fineract.apache.org> >>>>>>>> >>>>>>>> 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 < >>>>>>>> rrpawar141...@gmail.com> 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 <edca...@mifos.org> wrote: >>>>>>>>> >>>>>>>>>> Thank you for the update. >>>>>>>>>> >>>>>>>>>> On Wed, Jan 1, 2020, 07:02 RAHUL PAWAR <rrpawar141...@gmail.com> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Ed, >>>>>>>>>>> I am still working on testcases, will be completed soon. >>>>>>>>>>> >>>>>>>>>>> On Wed, Nov 13, 2019 at 7:23 AM Nikhil Pawar <nickr...@gmail.com> >>>>>>>>>>> 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 <edca...@mifos.org> >>>>>>>>>>>> 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 >>>>>>>> edca...@mifos.org | 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> >>>>>>>> >>>>>>>>
