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 #1488: FINERACT-1241-elastic-web-hook-update
Date Sat, 07 Nov 2020 16:11:11 GMT

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



##########
File path: fineract-provider/dependencies.gradle
##########
@@ -79,7 +79,8 @@ dependencies {
             'io.swagger.core.v3:swagger-annotations',
             'org.webjars:webjars-locator-core',
 
-            'com.google.cloud.sql:mysql-socket-factory-connector-j-8:1.1.0'
+            'com.google.cloud.sql:mysql-socket-factory-connector-j-8:1.1.0',
+            'com.squareup.retrofit2:converter-gson:2.1.0'

Review comment:
       @fynmanoj sorry I keep being a PITA here, but this `retrofit2:converter-gson:2.1.0`
here is not good - you shouldn't fix the version here - because we already fixed a version
in the root `fineract/build.gradle` (not `fineract-provider/` - but there we currently have
`2.9.0` instead of your old `2.1.0`.. would you be able to just drop the version number here
(it should work, and get "inherited", or was there a reason why you NEED `2.1.0` specifically?
   
   ```suggestion
               'com.squareup.retrofit2:converter-gson'
   ```

##########
File path: fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java
##########
@@ -125,7 +143,8 @@ public CommandProcessingResult processAndLogCommand(final CommandWrapper
wrapper
         }
         result.setRollbackTransaction(null);
 
-        publishEvent(wrapper.entityName(), wrapper.actionName(), result);
+        // publishEvent(wrapper.entityName(), wrapper.actionName(), result);
+        publishEvent(wrapper.entityName(), wrapper.actionName(), command, result);

Review comment:
       Just removing such lines entirely instead of leaving them commented out like this is
less "cluttered" (there is always git, for history):
   
   ```suggestion
           publishEvent(wrapper.entityName(), wrapper.actionName(), command, result);
   ```




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