You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/10/19 09:23:18 UTC
[GitHub] [fineract] vidakovic opened a new pull request #1432: FINERACT-1216: Add FineractClientTest for Fineract SDK REST Client (fix)
vidakovic opened a new pull request #1432:
URL: https://github.com/apache/fineract/pull/1432
@ptuomola @vorburger Could one of you quickly have a look at this and merge please? There are 2 small issues with Checkstyle that this PR fixes.
Builds are currently broken because of this one here.
----------------------------------------------------------------
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
[GitHub] [fineract] vorburger commented on a change in pull request #1432: FINERACT-1216: Add FineractClientTest for Fineract SDK REST Client (fix)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1432:
URL: https://github.com/apache/fineract/pull/1432#discussion_r507612549
##########
File path: fineract-client/src/main/java/org/apache/fineract/client/util/FineractClient.java
##########
@@ -26,7 +26,7 @@
*
* @author Michael Vorburger.ch
*/
-public class FineractClient {
+public final class FineractClient {
Review comment:
.. but this shouldn't be required? Especially since in #1433 I'm making `createService()` a protected method, so this class should remain _open_ (extends-ible).
##########
File path: fineract-client/src/main/java/org/apache/fineract/client/util/Calls.java
##########
@@ -27,7 +27,7 @@
*
* @author Michael Vorburger.ch
*/
-public class Calls {
+public final class Calls {
Review comment:
This makes sense, because it's a utility class with a private constructor... (and I vaguely remember that we have a Checkstyle or ErrorProne or SpotBugs check to enforce this - just want to find out why builds don't show up as red - or where they do)
##########
File path: fineract-client/src/main/java/org/apache/fineract/client/util/FineractClient.java
##########
@@ -42,7 +42,7 @@ public static FineractClientBuilder builder() {
return apiClient.createService(serviceClass);
}
- public static class FineractClientBuilder {
+ public static final class FineractClientBuilder {
Review comment:
as above (I think); best to see failing build and check if all 3 or only 1st (I think) need to be fixed?
----------------------------------------------------------------
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
[GitHub] [fineract] vorburger commented on pull request #1432: FINERACT-1216: Add FineractClientTest for Fineract SDK REST Client (fix)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1432:
URL: https://github.com/apache/fineract/pull/1432#issuecomment-712099329
> I'm curious if e.g. #1431 or #1433 will fail to build on Travis...
They passed, so what does this fix? :smiling_imp: But I have a suspicion that we may have a larger problem.. we don't have the same quality controls in `fineract-client` that we have in `fineract-provider` - do we?
----------------------------------------------------------------
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
[GitHub] [fineract] vidakovic closed pull request #1432: FINERACT-1216: Add FineractClientTest for Fineract SDK REST Client (fix)
Posted by GitBox <gi...@apache.org>.
vidakovic closed pull request #1432:
URL: https://github.com/apache/fineract/pull/1432
----------------------------------------------------------------
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
[GitHub] [fineract] vidakovic commented on pull request #1432: FINERACT-1216: Add FineractClientTest for Fineract SDK REST Client (fix)
Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #1432:
URL: https://github.com/apache/fineract/pull/1432#issuecomment-712532626
Already taken care of with #1447
----------------------------------------------------------------
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
[GitHub] [fineract] vorburger commented on pull request #1432: FINERACT-1216: Add FineractClientTest for Fineract SDK REST Client (fix)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1432:
URL: https://github.com/apache/fineract/pull/1432#issuecomment-711953952
Did you locally see a failure (running what?) for a task which we are missing on Travis?
I'm curious if e.g. #1431 or #1433 will fail to build on Travis...
----------------------------------------------------------------
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