fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] vorburger commented on a change in pull request #1050: FINERACT-1022 - Update to org.ehcache
Date Sat, 13 Jun 2020 09:02:44 GMT

vorburger commented on a change in pull request #1050:
URL: https://github.com/apache/fineract/pull/1050#discussion_r439721219



##########
File path: fineract-provider/build.gradle
##########
@@ -110,6 +110,7 @@ dependencyManagement {
         dependency 'io.github.classgraph:classgraph:4.8.85'
         dependency 'org.awaitility:awaitility:4.0.3'
         dependency 'com.github.spotbugs:spotbugs-annotations:4.0.4'
+        dependency 'javax.cache:cache-api:1.1.1'

Review comment:
       Isn't that a transitive dependency of ehCache already?

##########
File path: fineract-provider/build.gradle
##########
@@ -94,7 +94,7 @@ dependencyManagement {
         dependency 'org.flywaydb:flyway-core:6.4.4'
         dependency 'org.quartz-scheduler:quartz:2.3.2'
         dependency 'com.amazonaws:aws-java-sdk-s3:1.11.800'
-        dependency 'net.sf.ehcache:ehcache:2.10.6' // TODO FINERACT-1018
+        dependency 'org.ehcache:ehcache:3.8.1' // TODO FINERACT-1018

Review comment:
       rm TODO ?

##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/cache/PlatformCacheConfiguration.java
##########
@@ -18,6 +18,7 @@
  */
 package org.apache.fineract.infrastructure.cache;
 
+// import javax.cache.CacheManager;

Review comment:
       Don't add new commented out imports, but clean up

##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/cache/service/RuntimeDelegatingCacheManager.java
##########
@@ -120,6 +123,11 @@ public Cache getCache(final String name) {
     }
 
     private void clearEhCache() {
-        this.ehcacheCacheManager.getCacheManager().clearAll();
+        // this.jcacheCacheManager.getCacheManager().removeAll();

Review comment:
       rm? Is there no removeAll() on the new standard API?




----------------------------------------------------------------
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:
users@infra.apache.org



Mime
View raw message