adamsaghy commented on PR #5951:
URL: https://github.com/apache/fineract/pull/5951#issuecomment-5059701614

   > Kindly review the below concern:
   > 
   > ```
   > **1 blocking violation(s) found**.
   > These changes violate Liquibase migration safety rules and must be 
reviewed.
   > 
   > | Severity | File | Operation | Table | Risk |
   > |----------|------|-----------|-------|------|
   > | BLOCKING | 
`db/changelog/tenant/module/savings/parts/parts/2006_add_last_cob_business_date_to_savings_account.xml`
 | datetime-precision | `n/a (line: 43, match: type="DATETIME")` | Use 
DATETIME(6) instead of DATETIME; MySQL DATETIME defaults to precision 0 |
   > | INFO | 
`db/changelog/tenant/module/savings/parts/parts/2006_add_last_cob_business_date_to_savings_account.xml`
 | addColumn+NOT_NULL | `m_savings_account_locks` | Adding a NOT NULL column 
without default value breaks old code INSERTs (table created in same PR - 
likely safe) |
   > ```
   > 
   > Would it make sense to use common abstraction for unlocking locked 
accounts? (see below)
   > 
   > `UnlockProcessedSavingsTasklet` calls the JPA repository directly rather 
than going through the shared 
`AccountLockService`/`UnlockProcessedAccountsTasklet` that loan and 
working-capital-loan use. Functionally equivalent now (the query matches loan's 
semantics), but it's the one remaining spot that isn't fully sharing the common 
abstraction.
   
   @alberto-art3ch Please ping me once, i can rereview (we are getting quite 
close to be able to merge it)
   


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to