fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <>
Subject [GitHub] [fineract] vorburger commented on pull request #928: [WIP] Migrate ORM from OpenJPA to EclipseLink(Fineract 849)
Date Fri, 26 Jun 2020 11:19:05 GMT

vorburger commented on pull request #928:

   Wow. First of all, this is impressive. Congratulations.
   I have some concerns that the bulk replace of `save()` by `saveAndFlush()` could actually
lead to worse performance.
   About the disabled tests, I'm wondering whether we shouldn't fix those before merging this...
one of them in particular (FINERACT-1051) seems to have something to do re. some "Pre-Closure
maturity amount" being suddenly different, and nother one is a failing SQL (FINERACT-1050)
and the third seems to indicate regression with the scheduler (FINERACT-1048), which with
much pain we just fixed... it seems to me that if we merge this PR now, we knowingly break
Fineract again, no?
   If in the meantime you already wanted to get a sub-set of this PR merged just to avoid
future merge conflicts, e.g. those `JpaSystemException` (which is a Spring and not an EclipseLink
specific class) you had to add could make a separate standalone PR which we could already
merge before the rest of this (if you like, just a thought & suggestion).

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:

View raw message