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/07/04 15:47:52 UTC

[GitHub] [fineract] thesmallstar opened a new pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

thesmallstar opened a new pull request #1149:
URL: https://github.com/apache/fineract/pull/1149


   I have also changed the regex in checkstyle.xml to support single-letter variables.
   Since:
   int I; // was not supported :( 


----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453963723



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       This probably is a side-effect of mass find-replace, thank you for finding this, updating. 




----------------------------------------------------------------
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 #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453984306



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -4843,7 +4843,7 @@ public void testLoanRefundByTransferCashBasedAccounting() {
         // refund 20 means paid: principal 1980, interest 240, fees 50, penalty
         // 0
 
-        Float TRANSFER_AMOUNT = 20f;
+        Float transferAmountValue = 20f;

Review comment:
       Ah ok fine, then ignore this (I'm doing some late night review on my mobile phone)




----------------------------------------------------------------
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] thesmallstar commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-657831575


   @vorburger What have I done here is:
   1. If a variable is not changed after initialisation I have made it final, and then it allows XYZ_ABC type of naming style.
   2. If a variable is changed and we cannot make it final, then we need to have to follow camelcasing (abcXyz) . 
   
   So, if a var is not changed I have made it final. Else I have forced it to follow the required regex. 
   
   Since we want only static final to have XYZ_ABC naming, I will make every name ( final/non-final) to follow camelcasing. 
   
   


----------------------------------------------------------------
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] thesmallstar commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-657024449


   @xurror  ready for review. 


----------------------------------------------------------------
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] thesmallstar commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-653786786


   failed due to connection error, closing and reopening this. 


----------------------------------------------------------------
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] xurror commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
xurror commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453321228



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       Did this really need to be renamed? Seems like FOO_BAR format worked pretty well on other local variables.
   There are other variables you've renamed here too. I think the changes are cool. Maybe just clarify me on this rename.




----------------------------------------------------------------
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] thesmallstar commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-653947886


   @xurror  donot merge this yet, another way to fix this violation was to add a final keyword, which would fix most of this violations without change in variable name.
   
   On the other hand, AbbreviationAsWordInName did not ignore final variable and did not allow XYZ_ABC format.
   
   Currently I am looking into AbbreviationAsWordInName's property to ignore final and then we can decide on what would be the best thing to do.
   
   IMO, we should instead just add the final keyword which would fix nearly all this violations and this variables dont change the value once assigned.


----------------------------------------------------------------
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] xurror commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-653842439


   No problem. Resolve merge conflicts and I can merge this.


----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453982182



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -4843,7 +4843,7 @@ public void testLoanRefundByTransferCashBasedAccounting() {
         // refund 20 means paid: principal 1980, interest 240, fees 50, penalty
         // 0
 
-        Float TRANSFER_AMOUNT = 20f;
+        Float transferAmountValue = 20f;

Review comment:
       No it is not, it is changed a few lines later 
   https://github.com/apache/fineract/blob/acd9de407f29cf1ba90f682b13fc03c55916f4e2/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java#L4887




----------------------------------------------------------------
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 #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453975083



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       I actually think this was right (good), because all upper case variable names (on Java), by convention, are ONLY for true static final constants (which to this is not)..




----------------------------------------------------------------
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] thesmallstar closed pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar closed pull request #1149:
URL: https://github.com/apache/fineract/pull/1149


   


----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453988260



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -3195,7 +3195,7 @@ public void testLoanScheduleWithInterestRecalculation_WITH_REST_SAME_AS_REPAYMEN
         String prepayAmount = String.valueOf(prepayDetail.get("amount"));
         validateNumberForEqualWithMsg("verify pre-close amount", "3551.93", prepayAmount);
         todaysDate = Calendar.getInstance(Utils.getTimeZoneOfTenant());
-        String LOAN_REPAYMENT_DATE = dateFormat.format(todaysDate.getTime());
+        final String LOAN_REPAYMENT_DATE = dateFormat.format(todaysDate.getTime());

Review comment:
       agreed, changing. 




----------------------------------------------------------------
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 #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453977085



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -3195,7 +3195,7 @@ public void testLoanScheduleWithInterestRecalculation_WITH_REST_SAME_AS_REPAYMEN
         String prepayAmount = String.valueOf(prepayDetail.get("amount"));
         validateNumberForEqualWithMsg("verify pre-close amount", "3551.93", prepayAmount);
         todaysDate = Calendar.getInstance(Utils.getTimeZoneOfTenant());
-        String LOAN_REPAYMENT_DATE = dateFormat.format(todaysDate.getTime());
+        final String LOAN_REPAYMENT_DATE = dateFormat.format(todaysDate.getTime());

Review comment:
       I would actually make this lower case, because it's not a constant.

##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/AccountingScenarioIntegrationTest.java
##########
@@ -802,10 +802,10 @@ public void checkPeriodicAccrualAccountingTillCurrentDateFlow() throws Interrupt
 
         float totalInterest = (float) loanSchedule.get(1).get("interestOriginalDue");
         DecimalFormat numberFormat = new DecimalFormat("#.00", new DecimalFormatSymbols(Locale.US));
-        float INTEREST_4_DAYS = totalInterest / totalDaysInPeriod * 4;
-        INTEREST_4_DAYS = Float.valueOf(numberFormat.format(INTEREST_4_DAYS));
+        float interest4Days = totalInterest / totalDaysInPeriod * 4;
+        interest4Days = Float.valueOf(numberFormat.format(interest4Days));

Review comment:
       I don't understand what this does (why it's needed) - but it has nothing to do with the Checkstyle change..

##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -4843,7 +4843,7 @@ public void testLoanRefundByTransferCashBasedAccounting() {
         // refund 20 means paid: principal 1980, interest 240, fees 50, penalty
         // 0
 
-        Float TRANSFER_AMOUNT = 20f;
+        Float transferAmountValue = 20f;

Review comment:
       If this is a real constant, make it final and upper case?




----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453987556



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       Ok agreed, making the changes. 




----------------------------------------------------------------
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] thesmallstar closed pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar closed pull request #1149:
URL: https://github.com/apache/fineract/pull/1149


   


----------------------------------------------------------------
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 #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453985252



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/AccountingScenarioIntegrationTest.java
##########
@@ -802,10 +802,10 @@ public void checkPeriodicAccrualAccountingTillCurrentDateFlow() throws Interrupt
 
         float totalInterest = (float) loanSchedule.get(1).get("interestOriginalDue");
         DecimalFormat numberFormat = new DecimalFormat("#.00", new DecimalFormatSymbols(Locale.US));
-        float INTEREST_4_DAYS = totalInterest / totalDaysInPeriod * 4;
-        INTEREST_4_DAYS = Float.valueOf(numberFormat.format(INTEREST_4_DAYS));
+        float interest4Days = totalInterest / totalDaysInPeriod * 4;
+        interest4Days = Float.valueOf(numberFormat.format(interest4Days));

Review comment:
       No I just meant the Float value format thing in the 2nd line that looks weird, to me.




----------------------------------------------------------------
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] thesmallstar commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-653932387


   > I have also changed the regex in checkstyle.xml to support single-letter variables.
   > Since:
   > int i; // was not supported :(
   
   -> @xurror  @awasum  any contradicting thoughts on this?


----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453976595



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       @vorburger  in that case:
   This would clash with AbbreviationAsWordInName checkstyle which only allows up to 4 capital letters in final variables. 
   What should we go with? 




----------------------------------------------------------------
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 merged pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger merged pull request #1149:
URL: https://github.com/apache/fineract/pull/1149


   


----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453976595



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       Edit: I am speaking wrong here, please ignore. -> See next comment. 
   
   @vorburger  in that case:
   This would clash with AbbreviationAsWordInName checkstyle which only allows up to 4 capital letters in final variables. 
   What should we go with? 




----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453979703



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/AccountingScenarioIntegrationTest.java
##########
@@ -802,10 +802,10 @@ public void checkPeriodicAccrualAccountingTillCurrentDateFlow() throws Interrupt
 
         float totalInterest = (float) loanSchedule.get(1).get("interestOriginalDue");
         DecimalFormat numberFormat = new DecimalFormat("#.00", new DecimalFormatSymbols(Locale.US));
-        float INTEREST_4_DAYS = totalInterest / totalDaysInPeriod * 4;
-        INTEREST_4_DAYS = Float.valueOf(numberFormat.format(INTEREST_4_DAYS));
+        float interest4Days = totalInterest / totalDaysInPeriod * 4;
+        interest4Days = Float.valueOf(numberFormat.format(interest4Days));

Review comment:
       LocalVariableName would only allow the name to have following type of regex: ^[a-z][a-z0-9][a-zA-Z0-9]*$|[a-z]




----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453979703



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/AccountingScenarioIntegrationTest.java
##########
@@ -802,10 +802,10 @@ public void checkPeriodicAccrualAccountingTillCurrentDateFlow() throws Interrupt
 
         float totalInterest = (float) loanSchedule.get(1).get("interestOriginalDue");
         DecimalFormat numberFormat = new DecimalFormat("#.00", new DecimalFormatSymbols(Locale.US));
-        float INTEREST_4_DAYS = totalInterest / totalDaysInPeriod * 4;
-        INTEREST_4_DAYS = Float.valueOf(numberFormat.format(INTEREST_4_DAYS));
+        float interest4Days = totalInterest / totalDaysInPeriod * 4;
+        interest4Days = Float.valueOf(numberFormat.format(interest4Days));

Review comment:
       LocalVariableName would only allow the name (local var,unless final) to have following type of regex: ^[a-z][a-z0-9][a-zA-Z0-9]*$|[a-z]




----------------------------------------------------------------
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] thesmallstar commented on a change in pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#discussion_r453976595



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -257,22 +257,22 @@ public void testSavingsAccount_WITH_ENFORCE_MIN_BALANCE() {
 
         DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy");
         Calendar todaysDate = Calendar.getInstance();
-        final String TRANSACTION_DATE = dateFormat.format(todaysDate.getTime());
+        final String transactionDateValue = dateFormat.format(todaysDate.getTime());

Review comment:
       Edit: I am speaking wrong here, please ignore.
   
   @vorburger  in that case:
   This would clash with AbbreviationAsWordInName checkstyle which only allows up to 4 capital letters in final variables. 
   What should we go with? 




----------------------------------------------------------------
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] thesmallstar commented on pull request #1149: FINERACT-821 Added and Enforced LocalVariableName Checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1149:
URL: https://github.com/apache/fineract/pull/1149#issuecomment-653791704


   I know it is not a very nice idea to close and reopen again :( but build failed again due to connection error.


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