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/08/26 15:42:55 UTC
[GitHub] [fineract] thesmallstar opened a new pull request #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
thesmallstar opened a new pull request #1290:
URL: https://github.com/apache/fineract/pull/1290
this PR enforces AbbreviationAsWordInName checkstyle, now only constantName checkstyle for "naming" is left.
A detailed review would help, even if it takes time and we don't merge this quickly, since there are a lot of changes.
I have suppressed the checkstyle at a few places, where I thought either this check style is not really useful/added unnecessary work.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491846195
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/GlobalConfigurationHelper.java
##########
@@ -41,18 +41,17 @@ public GlobalConfigurationHelper(final RequestSpecification requestSpec, final R
public static ArrayList<HashMap> getAllGlobalConfigurations(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec) {
- final String GET_ALL_GLOBAL_CONFIG_URL = "/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
+ final String getAllGlobalConfigURL = "/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING ALL GLOBAL CONFIGURATIONS -------------------------");
- final HashMap<String, ArrayList<HashMap>> response = Utils.performServerGet(requestSpec, responseSpec, GET_ALL_GLOBAL_CONFIG_URL,
- "");
+ final HashMap<String, ArrayList<HashMap>> response = Utils.performServerGet(requestSpec, responseSpec, getAllGlobalConfigURL, "");
return response.get("globalConfiguration");
}
public static HashMap getGlobalConfigurationById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String configId) {
- final String GET_GLOBAL_CONFIG_BY_ID_URL = "/fineract-provider/api/v1/configurations/" + configId + "?" + Utils.TENANT_IDENTIFIER;
+ final String getGlobalConfigByIDurl = "/fineract-provider/api/v1/configurations/" + configId + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING GLOBAL CONFIGURATION BY ID -------------------------");
Review comment:
updated
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-716391290
Closing this one to keep the PRs list clean, will reopen with the update soon. Cannot find any free time in the last few days.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-691527360
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r492973111
##########
File path: fineract-provider/src/main/java/org/apache/fineract/useradministration/api/RolesApiResourceSwagger.java
##########
@@ -24,7 +24,7 @@
/**
* Created by sanyam on 23/8/17.
*/
-@SuppressWarnings({ "MemberName" })
+@SuppressWarnings({ "MemberName", "AbbreviationAsWordInName" })
Review comment:
here it's probably OK, because you can't change it, as it's already part of the external Swagger API? But then it's worth to document it like this:
```suggestion
// Cannot change names of public API without breaking backwards compatibility
@SuppressWarnings({ "MemberName", "AbbreviationAsWordInName" })
```
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -60,7 +60,7 @@
/**
* Client Savings Integration Test for checking Savings Application.
*/
-@SuppressWarnings({ "rawtypes" })
+@SuppressWarnings({ "rawtypes", "AbbreviationAsWordInName" })
Review comment:
are there many problems in this test, would it be hard to also fix this, just so that it's nice?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -65,7 +65,7 @@
* Client Loan Integration Test for checking Loan Application Repayments Schedule, loan charges, penalties, loan
* repayments and verifying accounting transactions
*/
-@SuppressWarnings({ "rawtypes", "unchecked" })
+@SuppressWarnings({ "rawtypes", "unchecked", "AbbreviationAsWordInName" })
public class ClientLoanIntegrationTest {
Review comment:
@thesmallstar I would actually have to agree with @ptuomola that if just a little bit more work, it would be nice to finish this up.. but it depends on the effort, of course. How about we agree that we don't use `@SuppressWarnings("AbbreviationAsWordInName")` at least in the 2-3 `main` (non-`test`) code places I pointed out in this review, but that if there is any `test` code where it's an unreasonable amount of work to change it, we merge it anyway - would that seem fair?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/service/ExternalServicesConstants.java
##########
@@ -21,6 +21,7 @@
import java.util.HashSet;
import java.util.Set;
+@SuppressWarnings({ "AbbreviationAsWordInName" })
Review comment:
it would be nice to fix this as well.. I'm hoping Checkstyle is OK with `S3_SERVICE_NAME` (because those truly are global constants), but it probably just doesn't like `ExternalservicePropertiesJSONinputParams` and the others like it? Again, just refactor those to be e.g. `ExternalservicePropertiesJsonInputParams`, that's better, and more consistent.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/GroupSavingsIntegrationTest.java
##########
@@ -53,7 +53,7 @@
/**
* Group Savings Integration Test for checking Savings Application.
*/
-@SuppressWarnings({ "rawtypes", "unused" })
+@SuppressWarnings({ "rawtypes", "unused", "AbbreviationAsWordInName" })
Review comment:
as above, would it be hard to fix this also here?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainService.java
##########
@@ -21,6 +21,7 @@
import java.util.Date;
import org.apache.fineract.infrastructure.cache.domain.CacheType;
+@SuppressWarnings({ "AbbreviationAsWordInName" })
Review comment:
this is not test code, so it should be fixed.. is it because of `isSMSOTPDeliveryEnabled` etc.? Just make that e.g. `isSmsOtpDeliveryEnabled` - it's fine, and actually more not less readable (once you are used to the "convention" - and it's better to have it "uniform" across the entire code base).
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r492973111
##########
File path: fineract-provider/src/main/java/org/apache/fineract/useradministration/api/RolesApiResourceSwagger.java
##########
@@ -24,7 +24,7 @@
/**
* Created by sanyam on 23/8/17.
*/
-@SuppressWarnings({ "MemberName" })
+@SuppressWarnings({ "MemberName", "AbbreviationAsWordInName" })
Review comment:
here it's probably OK, because you can't change it, as it's already part of the external Swagger API? But then it's worth to document it like this:
```suggestion
// Cannot change names of public API without breaking backwards compatibility
@SuppressWarnings({ "MemberName", "AbbreviationAsWordInName" })
```
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -60,7 +60,7 @@
/**
* Client Savings Integration Test for checking Savings Application.
*/
-@SuppressWarnings({ "rawtypes" })
+@SuppressWarnings({ "rawtypes", "AbbreviationAsWordInName" })
Review comment:
are there many problems in this test, would it be hard to also fix this, just so that it's nice?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -65,7 +65,7 @@
* Client Loan Integration Test for checking Loan Application Repayments Schedule, loan charges, penalties, loan
* repayments and verifying accounting transactions
*/
-@SuppressWarnings({ "rawtypes", "unchecked" })
+@SuppressWarnings({ "rawtypes", "unchecked", "AbbreviationAsWordInName" })
public class ClientLoanIntegrationTest {
Review comment:
@thesmallstar I would actually have to agree with @ptuomola that if just a little bit more work, it would be nice to finish this up.. but it depends on the effort, of course. How about we agree that we don't use `@SuppressWarnings("AbbreviationAsWordInName")` at least in the 2-3 `main` (non-`test`) code places I pointed out in this review, but that if there is any `test` code where it's an unreasonable amount of work to change it, we merge it anyway - would that seem fair?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/service/ExternalServicesConstants.java
##########
@@ -21,6 +21,7 @@
import java.util.HashSet;
import java.util.Set;
+@SuppressWarnings({ "AbbreviationAsWordInName" })
Review comment:
it would be nice to fix this as well.. I'm hoping Checkstyle is OK with `S3_SERVICE_NAME` (because those truly are global constants), but it probably just doesn't like `ExternalservicePropertiesJSONinputParams` and the others like it? Again, just refactor those to be e.g. `ExternalservicePropertiesJsonInputParams`, that's better, and more consistent.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/GroupSavingsIntegrationTest.java
##########
@@ -53,7 +53,7 @@
/**
* Group Savings Integration Test for checking Savings Application.
*/
-@SuppressWarnings({ "rawtypes", "unused" })
+@SuppressWarnings({ "rawtypes", "unused", "AbbreviationAsWordInName" })
Review comment:
as above, would it be hard to fix this also here?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainService.java
##########
@@ -21,6 +21,7 @@
import java.util.Date;
import org.apache.fineract.infrastructure.cache.domain.CacheType;
+@SuppressWarnings({ "AbbreviationAsWordInName" })
Review comment:
this is not test code, so it should be fixed.. is it because of `isSMSOTPDeliveryEnabled` etc.? Just make that e.g. `isSmsOtpDeliveryEnabled` - it's fine, and actually more not less readable (once you are used to the "convention" - and it's better to have it "uniform" across the entire code base).
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491847373
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -3117,7 +3117,7 @@ public void testLoanScheduleWithInterestRecalculation_WITH_REST_SAME_AS_REPAYMEN
Calendar todaysDate = Calendar.getInstance(Utils.getTimeZoneOfTenant());
todaysDate.add(Calendar.DAY_OF_MONTH, -14);
- final String LOAN_DISBURSEMENT_DATE = dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue = dateFormat.format(todaysDate.getTime());
Review comment:
Updated
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-691273960
@thesmallstar friendly ping, will you finish this one off?
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-706177011
@vorburger sorry for the delay, I should get this done by coming Monday. I will have ended all my submissions by Sunday :)
-Thanks
Manthan
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-696944678
Thanks for the review, Let us not merge this yet, I will complete the remaining files :)
I agree It wont take much effort, and it makes more sense to get this completely done RN. :D
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491843975
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositAccountHelper.java
##########
@@ -144,9 +144,9 @@ public static Integer applyFixedDepositApplication(final String fixedDepositAcco
public static HashMap getFixedDepositAccountById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_FIXED_DEPOSIT_BY_ID_URL = FIXED_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getFixedDepositByIDurl = FIXED_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT ACCOUNT BY ID -------------------------");
Review comment:
agreed, updated.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-701044752
Will be looking into this, and other issues this week end :)
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar closed pull request #1290:
URL: https://github.com/apache/fineract/pull/1290
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491847257
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/MinimumDaysBetweenDisbursalAndFirstRepaymentTest.java
##########
@@ -43,7 +43,7 @@
/**
* Test the creation, approval and rejection of a loan reschedule request
**/
-@SuppressWarnings({ "rawtypes" })
+@SuppressWarnings({ "rawtypes", "AbbreviationAsWordInName" })
public class MinimumDaysBetweenDisbursalAndFirstRepaymentTest {
Review comment:
Agreed, Removed.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/GroupSavingsIntegrationTest.java
##########
@@ -160,10 +160,10 @@ public void testSavingsAccount_CLOSE_APPLICATION() {
DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy", Locale.US);
Calendar todaysDate = Calendar.getInstance();
- final String CLOSEDON_DATE = dateFormat.format(todaysDate.getTime());
+ final String closedOnDateValue = dateFormat.format(todaysDate.getTime());
String withdrawBalance = "false";
Review comment:
updated.
----------------------------------------------------------------
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] ptuomola commented on a change in pull request #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r480686680
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/organisation/CampaignsHelper.java
##########
@@ -53,50 +53,50 @@ public CampaignsHelper(final RequestSpecification requestSpec, final ResponseSpe
public Integer createCampaign(String reportName, Integer triggerType) {
LOG.info("---------------------------------CREATING A CAMPAIGN---------------------------------------------");
- final String CREATE_SMS_CAMPAIGNS_URL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec, CREATE_SMS_CAMPAIGNS_URL, getCreateCampaignJSON(reportName, triggerType),
+ final String createSMScampaignsURL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
We've made SMS lowercase elsewhere, so would these make more sense as createSmsCampaignsURL etc?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/EmbeddedTomcatWithSSLConfiguration.java
##########
@@ -55,7 +55,7 @@ protected Connector createSslConnector() {
File keystore = getFile(getKeystore());
connector.setScheme("https");
connector.setSecure(true);
- connector.setPort(getHTTPSPort());
+ connector.setPort(getHTTPSport());
Review comment:
Would getHttpsPort not make more sense - now it looks like getHttpSport :-)
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/SchedulerJobHelper.java
##########
@@ -76,32 +76,32 @@ public SchedulerJobHelper(final RequestSpecification requestSpec) {
}
public Map<String, Object> getSchedulerJobById(int jobId) {
- final String GET_SCHEDULER_JOB_BY_ID_URL = "/fineract-provider/api/v1/jobs/" + jobId + "?" + Utils.TENANT_IDENTIFIER;
+ final String getSchedularJobBYIDurl = "/fineract-provider/api/v1/jobs/" + jobId + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING SCHEDULER JOB BY ID -------------------------");
Review comment:
I think this should be getSchedulerJobByIdURL
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/LoanWithWaiveInterestAndWriteOffIntegrationTest.java
##########
@@ -140,7 +140,7 @@ public void checkClientLoanCreateAndDisburseFlow() {
}
@Test
- public void checkClientLoan_WRITTEN_OFF() {
+ public void checkClientLoanWrittenOFF() {
Review comment:
I don't think OFF is an abbreviation - so if we use CamelCase, let's do it for the whole word (even if CheckStyle would allow us not to)
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/LoanWithWaiveInterestAndWriteOffIntegrationTest.java
##########
@@ -140,7 +140,7 @@ public void checkClientLoanCreateAndDisburseFlow() {
}
@Test
- public void checkClientLoan_WRITTEN_OFF() {
+ public void checkClientLoanWrittenOFF() {
// CREATE CLIENT
Review comment:
OFF is not a constant so shouldn't this be checkClientLoanWrittenOff
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/GroupSavingsIntegrationTest.java
##########
@@ -160,10 +160,10 @@ public void testSavingsAccount_CLOSE_APPLICATION() {
DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy", Locale.US);
Calendar todaysDate = Calendar.getInstance();
- final String CLOSEDON_DATE = dateFormat.format(todaysDate.getTime());
+ final String closedOnDateValue = dateFormat.format(todaysDate.getTime());
String withdrawBalance = "false";
Review comment:
Is there a reason for adding ...Value for this specifically?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositAccountHelper.java
##########
@@ -156,9 +156,9 @@ public static Integer applyRecurringDepositApplication(final String recurringDep
public static HashMap getRecurringDepositAccountById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_RECURRING_DEPOSIT_BY_ID_URL = RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getRecurringDepositByIDurl = RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
Would getRecurringDepositByIdURL be more consistent with the others that capitalise URL?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/group/api/GroupsApiResource.java
##########
@@ -624,11 +624,11 @@ public String retrieveGsimAccounts(@PathParam("groupId") final Long groupId,
}
- final Set<String> GSIM_ACCOUNTS_DATA_PARAMETERS = new HashSet<>(
+ final Set<String> glimAccountsDataParameters = new HashSet<>(
Arrays.asList("gsimId", "groupId", "accountNumber", "childGSIMAccounts", "parentBalance", "savingsStatus"));
Review comment:
Mistake here I think - shouldn't this be gsim not glim - as in the original code?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositProductHelper.java
##########
@@ -244,8 +244,8 @@ public static ArrayList retrieveAllRecurringDepositProducts(final RequestSpecifi
public static HashMap retrieveRecurringDepositProductById(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec, final String productId) {
LOG.info("-------------------- RETRIEVING RECURRING DEPOSIT PRODUCT BY ID --------------------------");
- final String GET_RD_PRODUCT_BY_ID_URL = RECURRING_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec, responseSpec, GET_RD_PRODUCT_BY_ID_URL, "");
+ final String getRDproductByIDurl = RECURRING_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
Would getRdProductByIdURL be more consistent with the others?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/HolidayHelper.java
##########
@@ -77,15 +77,15 @@ public static Integer createHolidays(final RequestSpecification requestSpec, fin
public static Integer activateHolidays(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String ACTIVATE_HOLIDAY_URL = HOLIDAYS_URL + "/" + holidayID + "?command=activate&" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec, ACTIVATE_HOLIDAY_URL, getActivateHolidayDataAsJSON(), "resourceId");
+ final String activateHolidayURL = HOLIDAYS_URL + "/" + holidayID + "?command=activate&" + Utils.TENANT_IDENTIFIER;
+ return Utils.performServerPost(requestSpec, responseSpec, activateHolidayURL, getActivateHolidayDataAsJSON(), "resourceId");
}
public static HashMap getHolidayById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String GET_HOLIDAY_BY_ID_URL = HOLIDAYS_URL + "/" + holidayID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getHolidayByIDurl = HOLIDAYS_URL + "/" + holidayID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING HOLIDAY BY ID -------------------------");
Review comment:
Would be good to be consistent with capitalising URL - so this would be getHolidayByIdURL
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/loans/LoanTransactionHelper.java
##########
@@ -303,46 +303,46 @@ public HashMap withdrawLoanApplicationByClient(final String date, final Integer
public Integer addChargesForLoan(final Integer loanId, final String request) {
LOG.info("--------------------------------- ADD CHARGES FOR LOAN --------------------------------");
- final String ADD_CHARGES_URL = "/fineract-provider/api/v1/loans/" + loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerPost(requestSpec, responseSpec, ADD_CHARGES_URL, request, "");
+ final String chargesURL = "/fineract-provider/api/v1/loans/" + loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
Review comment:
Is there a reason for dropping the add from the name? Why not addChargesURL?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositProductHelper.java
##########
@@ -436,8 +436,8 @@ public static ArrayList retrieveAllFixedDepositProducts(final RequestSpecificati
public static HashMap retrieveFixedDepositProductById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String productId) {
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT PRODUCT BY ID ------------------------");
- final String GET_FD_PRODUCT_BY_ID_URL = FIXED_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec, responseSpec, GET_FD_PRODUCT_BY_ID_URL, "");
+ final String getFDproductByIDurl = FIXED_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
+ final HashMap response = Utils.performServerGet(requestSpec, responseSpec, getFDproductByIDurl, "");
Review comment:
I think cleaner and more consistent would be getFdProductByIdURL - what do you think?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositAccountHelper.java
##########
@@ -144,9 +144,9 @@ public static Integer applyFixedDepositApplication(final String fixedDepositAcco
public static HashMap getFixedDepositAccountById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_FIXED_DEPOSIT_BY_ID_URL = FIXED_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getFixedDepositByIDurl = FIXED_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT ACCOUNT BY ID -------------------------");
Review comment:
Would be good to be consistent in capitalising URL - so this would be getFixedDepositByIdURL
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -65,7 +65,7 @@
* Client Loan Integration Test for checking Loan Application Repayments Schedule, loan charges, penalties, loan
* repayments and verifying accounting transactions
*/
-@SuppressWarnings({ "rawtypes", "unchecked" })
+@SuppressWarnings({ "rawtypes", "unchecked", "AbbreviationAsWordInName" })
public class ClientLoanIntegrationTest {
Review comment:
What was the reason for suppressing for this file?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/StandingInstructionsHelper.java
##########
@@ -108,20 +108,20 @@ public Integer createStandingInstruction(final String clientId, final String fro
public HashMap getStandingInstructionById(final String standingInstructionId) {
LOG.info("----------------------------- RETRIEVING STANDING INSTRUCTION BY ID---------------------------");
- final String GET_STANDING_INSTRUCTION_BY_ID_URL = STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ final String getStandingInstructionsByIDurl = STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ Utils.TENANT_IDENTIFIER;
Review comment:
Would be good to be consistent in capitalising URL - so this would be getStandingInstructionsByIdURL
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/MinimumDaysBetweenDisbursalAndFirstRepaymentTest.java
##########
@@ -43,7 +43,7 @@
/**
* Test the creation, approval and rejection of a loan reschedule request
**/
-@SuppressWarnings({ "rawtypes" })
+@SuppressWarnings({ "rawtypes", "AbbreviationAsWordInName" })
public class MinimumDaysBetweenDisbursalAndFirstRepaymentTest {
Review comment:
What was the reason for suppressing for this file? I didn't see anything specific why we would not be able to use... but perhaps I missed something
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/GlobalConfigurationHelper.java
##########
@@ -41,18 +41,17 @@ public GlobalConfigurationHelper(final RequestSpecification requestSpec, final R
public static ArrayList<HashMap> getAllGlobalConfigurations(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec) {
- final String GET_ALL_GLOBAL_CONFIG_URL = "/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
+ final String getAllGlobalConfigURL = "/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING ALL GLOBAL CONFIGURATIONS -------------------------");
- final HashMap<String, ArrayList<HashMap>> response = Utils.performServerGet(requestSpec, responseSpec, GET_ALL_GLOBAL_CONFIG_URL,
- "");
+ final HashMap<String, ArrayList<HashMap>> response = Utils.performServerGet(requestSpec, responseSpec, getAllGlobalConfigURL, "");
return response.get("globalConfiguration");
}
public static HashMap getGlobalConfigurationById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String configId) {
- final String GET_GLOBAL_CONFIG_BY_ID_URL = "/fineract-provider/api/v1/configurations/" + configId + "?" + Utils.TENANT_IDENTIFIER;
+ final String getGlobalConfigByIDurl = "/fineract-provider/api/v1/configurations/" + configId + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING GLOBAL CONFIGURATION BY ID -------------------------");
Review comment:
Would be good to be consistent at capitalising URL - so this would be getGlobalConfigByIdURL
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/SchedulerJobsTestResults.java
##########
@@ -537,11 +537,11 @@ public void testUpdateLoanSummaryJobOutcome() throws InterruptedException {
final String currentDate = dateFormat.format(todaysDate.getTime());
todaysDate.add(Calendar.MONTH, -1);
- final String LOAN_DISBURSEMENT_DATE = dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue = dateFormat.format(todaysDate.getTime());
Review comment:
Is there a reason why we have ...Value added to some (but not all) of these?
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -3117,7 +3117,7 @@ public void testLoanScheduleWithInterestRecalculation_WITH_REST_SAME_AS_REPAYMEN
Calendar todaysDate = Calendar.getInstance(Utils.getTimeZoneOfTenant());
todaysDate.add(Calendar.DAY_OF_MONTH, -14);
- final String LOAN_DISBURSEMENT_DATE = dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue = dateFormat.format(todaysDate.getTime());
Review comment:
This one has ...Value, others don't. Is there a difference that you had in mind?
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
xurror commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r478873697
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/RecurringDepositTest.java
##########
@@ -2774,11 +2774,11 @@ public void testRecurringDepositQuarterlyCompoundingAndQuarterlyPosting_360_Days
Assertions.assertNotNull(recurringDepositProductId);
Integer recurringDepositAccountId = applyForRecurringDepositApplication(clientId.toString(), recurringDepositProductId.toString(),
- VALID_FROM, VALID_TO, SUBMITTED_ON_DATE, WHOLE_TERM, expectedFirstDepositOnDate);
+ VALID_FROM, VALID_TO, submittedOnDate, WHOLE_TERM, expectedFirstDepositOnDate);
Assertions.assertNotNull(recurringDepositAccountId);
HashMap modificationsHashMap = this.recurringDepositAccountHelper.updateInterestCalculationConfigForRecurringDeposit(
- clientId.toString(), recurringDepositProductId.toString(), recurringDepositAccountId.toString(), SUBMITTED_ON_DATE,
+ clientId.toString(), recurringDepositProductId.toString(), recurringDepositAccountId.toString(), submittedOnDate,
VALID_FROM, VALID_TO, DAYS_360, WHOLE_TERM, INTEREST_CALCULATION_USING_DAILY_BALANCE, QUARTERLY, QUARTERLY,
Review comment:
OK, makes sense.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-691273960
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491844647
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/StandingInstructionsHelper.java
##########
@@ -108,20 +108,20 @@ public Integer createStandingInstruction(final String clientId, final String fro
public HashMap getStandingInstructionById(final String standingInstructionId) {
LOG.info("----------------------------- RETRIEVING STANDING INSTRUCTION BY ID---------------------------");
- final String GET_STANDING_INSTRUCTION_BY_ID_URL = STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ final String getStandingInstructionsByIDurl = STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ Utils.TENANT_IDENTIFIER;
Review comment:
updated.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491842279
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/LoanWithWaiveInterestAndWriteOffIntegrationTest.java
##########
@@ -140,7 +140,7 @@ public void checkClientLoanCreateAndDisburseFlow() {
}
@Test
- public void checkClientLoan_WRITTEN_OFF() {
+ public void checkClientLoanWrittenOFF() {
Review comment:
fixed.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/EmbeddedTomcatWithSSLConfiguration.java
##########
@@ -55,7 +55,7 @@ protected Connector createSslConnector() {
File keystore = getFile(getKeystore());
connector.setScheme("https");
connector.setSecure(true);
- connector.setPort(getHTTPSPort());
+ connector.setPort(getHTTPSport());
Review comment:
fixed
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/group/api/GroupsApiResource.java
##########
@@ -624,11 +624,11 @@ public String retrieveGsimAccounts(@PathParam("groupId") final Long groupId,
}
- final Set<String> GSIM_ACCOUNTS_DATA_PARAMETERS = new HashSet<>(
+ final Set<String> glimAccountsDataParameters = new HashSet<>(
Arrays.asList("gsimId", "groupId", "accountNumber", "childGSIMAccounts", "parentBalance", "savingsStatus"));
Review comment:
updated, thanks for finding this.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositProductHelper.java
##########
@@ -244,8 +244,8 @@ public static ArrayList retrieveAllRecurringDepositProducts(final RequestSpecifi
public static HashMap retrieveRecurringDepositProductById(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec, final String productId) {
LOG.info("-------------------- RETRIEVING RECURRING DEPOSIT PRODUCT BY ID --------------------------");
- final String GET_RD_PRODUCT_BY_ID_URL = RECURRING_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec, responseSpec, GET_RD_PRODUCT_BY_ID_URL, "");
+ final String getRDproductByIDurl = RECURRING_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositAccountHelper.java
##########
@@ -156,9 +156,9 @@ public static Integer applyRecurringDepositApplication(final String recurringDep
public static HashMap getRecurringDepositAccountById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_RECURRING_DEPOSIT_BY_ID_URL = RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getRecurringDepositByIDurl = RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/organisation/CampaignsHelper.java
##########
@@ -53,50 +53,50 @@ public CampaignsHelper(final RequestSpecification requestSpec, final ResponseSpe
public Integer createCampaign(String reportName, Integer triggerType) {
LOG.info("---------------------------------CREATING A CAMPAIGN---------------------------------------------");
- final String CREATE_SMS_CAMPAIGNS_URL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec, CREATE_SMS_CAMPAIGNS_URL, getCreateCampaignJSON(reportName, triggerType),
+ final String createSMScampaignsURL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/loans/LoanTransactionHelper.java
##########
@@ -303,46 +303,46 @@ public HashMap withdrawLoanApplicationByClient(final String date, final Integer
public Integer addChargesForLoan(final Integer loanId, final String request) {
LOG.info("--------------------------------- ADD CHARGES FOR LOAN --------------------------------");
- final String ADD_CHARGES_URL = "/fineract-provider/api/v1/loans/" + loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerPost(requestSpec, responseSpec, ADD_CHARGES_URL, request, "");
+ final String chargesURL = "/fineract-provider/api/v1/loans/" + loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated, I dont know what I was thinking at that time, probably a mistake.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositProductHelper.java
##########
@@ -436,8 +436,8 @@ public static ArrayList retrieveAllFixedDepositProducts(final RequestSpecificati
public static HashMap retrieveFixedDepositProductById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String productId) {
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT PRODUCT BY ID ------------------------");
- final String GET_FD_PRODUCT_BY_ID_URL = FIXED_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec, responseSpec, GET_FD_PRODUCT_BY_ID_URL, "");
+ final String getFDproductByIDurl = FIXED_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
+ final HashMap response = Utils.performServerGet(requestSpec, responseSpec, getFDproductByIDurl, "");
Review comment:
agreed, updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositAccountHelper.java
##########
@@ -144,9 +144,9 @@ public static Integer applyFixedDepositApplication(final String fixedDepositAcco
public static HashMap getFixedDepositAccountById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_FIXED_DEPOSIT_BY_ID_URL = FIXED_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getFixedDepositByIDurl = FIXED_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT ACCOUNT BY ID -------------------------");
Review comment:
agreed, updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/StandingInstructionsHelper.java
##########
@@ -108,20 +108,20 @@ public Integer createStandingInstruction(final String clientId, final String fro
public HashMap getStandingInstructionById(final String standingInstructionId) {
LOG.info("----------------------------- RETRIEVING STANDING INSTRUCTION BY ID---------------------------");
- final String GET_STANDING_INSTRUCTION_BY_ID_URL = STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ final String getStandingInstructionsByIDurl = STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ Utils.TENANT_IDENTIFIER;
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/HolidayHelper.java
##########
@@ -77,15 +77,15 @@ public static Integer createHolidays(final RequestSpecification requestSpec, fin
public static Integer activateHolidays(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String ACTIVATE_HOLIDAY_URL = HOLIDAYS_URL + "/" + holidayID + "?command=activate&" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec, ACTIVATE_HOLIDAY_URL, getActivateHolidayDataAsJSON(), "resourceId");
+ final String activateHolidayURL = HOLIDAYS_URL + "/" + holidayID + "?command=activate&" + Utils.TENANT_IDENTIFIER;
+ return Utils.performServerPost(requestSpec, responseSpec, activateHolidayURL, getActivateHolidayDataAsJSON(), "resourceId");
}
public static HashMap getHolidayById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String GET_HOLIDAY_BY_ID_URL = HOLIDAYS_URL + "/" + holidayID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getHolidayByIDurl = HOLIDAYS_URL + "/" + holidayID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING HOLIDAY BY ID -------------------------");
Review comment:
upated
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/GlobalConfigurationHelper.java
##########
@@ -41,18 +41,17 @@ public GlobalConfigurationHelper(final RequestSpecification requestSpec, final R
public static ArrayList<HashMap> getAllGlobalConfigurations(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec) {
- final String GET_ALL_GLOBAL_CONFIG_URL = "/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
+ final String getAllGlobalConfigURL = "/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING ALL GLOBAL CONFIGURATIONS -------------------------");
- final HashMap<String, ArrayList<HashMap>> response = Utils.performServerGet(requestSpec, responseSpec, GET_ALL_GLOBAL_CONFIG_URL,
- "");
+ final HashMap<String, ArrayList<HashMap>> response = Utils.performServerGet(requestSpec, responseSpec, getAllGlobalConfigURL, "");
return response.get("globalConfiguration");
}
public static HashMap getGlobalConfigurationById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String configId) {
- final String GET_GLOBAL_CONFIG_BY_ID_URL = "/fineract-provider/api/v1/configurations/" + configId + "?" + Utils.TENANT_IDENTIFIER;
+ final String getGlobalConfigByIDurl = "/fineract-provider/api/v1/configurations/" + configId + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING GLOBAL CONFIGURATION BY ID -------------------------");
Review comment:
updated
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/SchedulerJobsTestResults.java
##########
@@ -537,11 +537,11 @@ public void testUpdateLoanSummaryJobOutcome() throws InterruptedException {
final String currentDate = dateFormat.format(todaysDate.getTime());
todaysDate.add(Calendar.MONTH, -1);
- final String LOAN_DISBURSEMENT_DATE = dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue = dateFormat.format(todaysDate.getTime());
Review comment:
Updated, I did it since the variable loanDisbursementDate was used in some files, did not let me mass replace.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/MinimumDaysBetweenDisbursalAndFirstRepaymentTest.java
##########
@@ -43,7 +43,7 @@
/**
* Test the creation, approval and rejection of a loan reschedule request
**/
-@SuppressWarnings({ "rawtypes" })
+@SuppressWarnings({ "rawtypes", "AbbreviationAsWordInName" })
public class MinimumDaysBetweenDisbursalAndFirstRepaymentTest {
Review comment:
Agreed, Removed.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/GroupSavingsIntegrationTest.java
##########
@@ -160,10 +160,10 @@ public void testSavingsAccount_CLOSE_APPLICATION() {
DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy", Locale.US);
Calendar todaysDate = Calendar.getInstance();
- final String CLOSEDON_DATE = dateFormat.format(todaysDate.getTime());
+ final String closedOnDateValue = dateFormat.format(todaysDate.getTime());
String withdrawBalance = "false";
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -3117,7 +3117,7 @@ public void testLoanScheduleWithInterestRecalculation_WITH_REST_SAME_AS_REPAYMEN
Calendar todaysDate = Calendar.getInstance(Utils.getTimeZoneOfTenant());
todaysDate.add(Calendar.DAY_OF_MONTH, -14);
- final String LOAN_DISBURSEMENT_DATE = dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue = dateFormat.format(todaysDate.getTime());
Review comment:
Updated
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -65,7 +65,7 @@
* Client Loan Integration Test for checking Loan Application Repayments Schedule, loan charges, penalties, loan
* repayments and verifying accounting transactions
*/
-@SuppressWarnings({ "rawtypes", "unchecked" })
+@SuppressWarnings({ "rawtypes", "unchecked", "AbbreviationAsWordInName" })
public class ClientLoanIntegrationTest {
Review comment:
Only this remains, I suppressed it here since there were 130-140 letter big function names and many of them, I thought it was really easy to read them in the way they are now.
Also, I found those changes unnecessary since other errors in that file were solved already. test functions name should not matter much?
What do you think?
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
xurror commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r478707601
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/RecurringDepositTest.java
##########
@@ -2774,11 +2774,11 @@ public void testRecurringDepositQuarterlyCompoundingAndQuarterlyPosting_360_Days
Assertions.assertNotNull(recurringDepositProductId);
Integer recurringDepositAccountId = applyForRecurringDepositApplication(clientId.toString(), recurringDepositProductId.toString(),
- VALID_FROM, VALID_TO, SUBMITTED_ON_DATE, WHOLE_TERM, expectedFirstDepositOnDate);
+ VALID_FROM, VALID_TO, submittedOnDate, WHOLE_TERM, expectedFirstDepositOnDate);
Assertions.assertNotNull(recurringDepositAccountId);
HashMap modificationsHashMap = this.recurringDepositAccountHelper.updateInterestCalculationConfigForRecurringDeposit(
- clientId.toString(), recurringDepositProductId.toString(), recurringDepositAccountId.toString(), SUBMITTED_ON_DATE,
+ clientId.toString(), recurringDepositProductId.toString(), recurringDepositAccountId.toString(), submittedOnDate,
VALID_FROM, VALID_TO, DAYS_360, WHOLE_TERM, INTEREST_CALCULATION_USING_DAILY_BALANCE, QUARTERLY, QUARTERLY,
Review comment:
I think these variables could use the change to camel case too. They don't look like some standard abbreviation. What do you think? Unless I don't really get what this checkstyle is enforcing.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-696944678
Thanks for the review, Let us not merge this yet, I will complete the remaining files :)
I agree It wont take much effort, and it makes more sense to get this completely done RN. :D
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-691527360
@vorburger sorry for the delay, I am in between of my college mid-semester exams, I will continue working as soon as I am done with it(hopefully no sooner than 19th Sept).
Yes ! will finish this off without a doubt :)
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491843695
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositProductHelper.java
##########
@@ -436,8 +436,8 @@ public static ArrayList retrieveAllFixedDepositProducts(final RequestSpecificati
public static HashMap retrieveFixedDepositProductById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String productId) {
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT PRODUCT BY ID ------------------------");
- final String GET_FD_PRODUCT_BY_ID_URL = FIXED_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec, responseSpec, GET_FD_PRODUCT_BY_ID_URL, "");
+ final String getFDproductByIDurl = FIXED_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
+ final HashMap response = Utils.performServerGet(requestSpec, responseSpec, getFDproductByIDurl, "");
Review comment:
agreed, updated.
----------------------------------------------------------------
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 edited a comment on pull request #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar edited a comment on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-701044752
Will be looking into this, and other issues this week end :)
Edit: bust for a few more days(damn my college :( )
will look into this soon.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-706177011
@vorburger sorry for the delay, I should get this done by coming Monday. I will have ended all my submissions by Sunday :)
-Thanks
Manthan
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491846979
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/SchedulerJobsTestResults.java
##########
@@ -537,11 +537,11 @@ public void testUpdateLoanSummaryJobOutcome() throws InterruptedException {
final String currentDate = dateFormat.format(todaysDate.getTime());
todaysDate.add(Calendar.MONTH, -1);
- final String LOAN_DISBURSEMENT_DATE = dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue = dateFormat.format(todaysDate.getTime());
Review comment:
Updated, I did it since the variable loanDisbursementDate was used in some files, did not let me mass replace.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491846062
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/HolidayHelper.java
##########
@@ -77,15 +77,15 @@ public static Integer createHolidays(final RequestSpecification requestSpec, fin
public static Integer activateHolidays(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String ACTIVATE_HOLIDAY_URL = HOLIDAYS_URL + "/" + holidayID + "?command=activate&" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec, ACTIVATE_HOLIDAY_URL, getActivateHolidayDataAsJSON(), "resourceId");
+ final String activateHolidayURL = HOLIDAYS_URL + "/" + holidayID + "?command=activate&" + Utils.TENANT_IDENTIFIER;
+ return Utils.performServerPost(requestSpec, responseSpec, activateHolidayURL, getActivateHolidayDataAsJSON(), "resourceId");
}
public static HashMap getHolidayById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String GET_HOLIDAY_BY_ID_URL = HOLIDAYS_URL + "/" + holidayID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getHolidayByIDurl = HOLIDAYS_URL + "/" + holidayID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING HOLIDAY BY ID -------------------------");
Review comment:
upated
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-682100056
@vorburger @ptuomola @xurror @awasum can you review this one?
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-687845113
@thesmallstar hope you saw @ptuomola review feedback on this PR and will have remember to update it when you have a moment? Tx!
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-706167779
@thesmallstar friendly ping? Maybe you'll find a minute for this over this weekend?
Or we could already just merge it as is, and you can raise a follow-up for final further clean-up.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-691527360
@vorburger sorry for the delay, I am in between of my college mid-semester exams, I will continue working as soon as I am done with it(hopefully no sooner than 19th Sept).
Yes ! will finish this off without a doubt :)
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r478860118
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/RecurringDepositTest.java
##########
@@ -2774,11 +2774,11 @@ public void testRecurringDepositQuarterlyCompoundingAndQuarterlyPosting_360_Days
Assertions.assertNotNull(recurringDepositProductId);
Integer recurringDepositAccountId = applyForRecurringDepositApplication(clientId.toString(), recurringDepositProductId.toString(),
- VALID_FROM, VALID_TO, SUBMITTED_ON_DATE, WHOLE_TERM, expectedFirstDepositOnDate);
+ VALID_FROM, VALID_TO, submittedOnDate, WHOLE_TERM, expectedFirstDepositOnDate);
Assertions.assertNotNull(recurringDepositAccountId);
HashMap modificationsHashMap = this.recurringDepositAccountHelper.updateInterestCalculationConfigForRecurringDeposit(
- clientId.toString(), recurringDepositProductId.toString(), recurringDepositAccountId.toString(), SUBMITTED_ON_DATE,
+ clientId.toString(), recurringDepositProductId.toString(), recurringDepositAccountId.toString(), submittedOnDate,
VALID_FROM, VALID_TO, DAYS_360, WHOLE_TERM, INTEREST_CALCULATION_USING_DAILY_BALANCE, QUARTERLY, QUARTERLY,
Review comment:
no this is how it should be, the other variables are static finals (constant) and hence they are named like that.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491843079
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/group/api/GroupsApiResource.java
##########
@@ -624,11 +624,11 @@ public String retrieveGsimAccounts(@PathParam("groupId") final Long groupId,
}
- final Set<String> GSIM_ACCOUNTS_DATA_PARAMETERS = new HashSet<>(
+ final Set<String> glimAccountsDataParameters = new HashSet<>(
Arrays.asList("gsimId", "groupId", "accountNumber", "childGSIMAccounts", "parentBalance", "savingsStatus"));
Review comment:
updated, thanks for finding this.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositProductHelper.java
##########
@@ -244,8 +244,8 @@ public static ArrayList retrieveAllRecurringDepositProducts(final RequestSpecifi
public static HashMap retrieveRecurringDepositProductById(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec, final String productId) {
LOG.info("-------------------- RETRIEVING RECURRING DEPOSIT PRODUCT BY ID --------------------------");
- final String GET_RD_PRODUCT_BY_ID_URL = RECURRING_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec, responseSpec, GET_RD_PRODUCT_BY_ID_URL, "");
+ final String getRDproductByIDurl = RECURRING_DEPOSIT_PRODUCT_URL + "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositAccountHelper.java
##########
@@ -156,9 +156,9 @@ public static Integer applyRecurringDepositApplication(final String recurringDep
public static HashMap getRecurringDepositAccountById(final RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_RECURRING_DEPOSIT_BY_ID_URL = RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getRecurringDepositByIDurl = RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated.
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/organisation/CampaignsHelper.java
##########
@@ -53,50 +53,50 @@ public CampaignsHelper(final RequestSpecification requestSpec, final ResponseSpe
public Integer createCampaign(String reportName, Integer triggerType) {
LOG.info("---------------------------------CREATING A CAMPAIGN---------------------------------------------");
- final String CREATE_SMS_CAMPAIGNS_URL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec, CREATE_SMS_CAMPAIGNS_URL, getCreateCampaignJSON(reportName, triggerType),
+ final String createSMScampaignsURL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-691273960
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r502972758
##########
File path: fineract-provider/src/main/java/org/apache/fineract/useradministration/api/RolesApiResourceSwagger.java
##########
@@ -24,7 +24,7 @@
/**
* Created by sanyam on 23/8/17.
*/
-@SuppressWarnings({ "MemberName" })
+@SuppressWarnings({ "MemberName", "AbbreviationAsWordInName" })
Review comment:
Yes agreed, 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] thesmallstar commented on a change in pull request #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491843455
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/loans/LoanTransactionHelper.java
##########
@@ -303,46 +303,46 @@ public HashMap withdrawLoanApplicationByClient(final String date, final Integer
public Integer addChargesForLoan(final Integer loanId, final String request) {
LOG.info("--------------------------------- ADD CHARGES FOR LOAN --------------------------------");
- final String ADD_CHARGES_URL = "/fineract-provider/api/v1/loans/" + loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerPost(requestSpec, responseSpec, ADD_CHARGES_URL, request, "");
+ final String chargesURL = "/fineract-provider/api/v1/loans/" + loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
Review comment:
updated, I dont know what I was thinking at that time, probably a mistake.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-706167779
@thesmallstar friendly ping? Maybe you'll find a minute for this over this weekend?
Or we could already just merge it as is, and you can raise a follow-up for final further clean-up.
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491848617
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -65,7 +65,7 @@
* Client Loan Integration Test for checking Loan Application Repayments Schedule, loan charges, penalties, loan
* repayments and verifying accounting transactions
*/
-@SuppressWarnings({ "rawtypes", "unchecked" })
+@SuppressWarnings({ "rawtypes", "unchecked", "AbbreviationAsWordInName" })
public class ClientLoanIntegrationTest {
Review comment:
Only this remains, I suppressed it here since there were 130-140 letter big function names and many of them, I thought it was really easy to read them in the way they are now.
Also, I found those changes unnecessary since other errors in that file were solved already. test functions name should not matter much?
What do you think?
----------------------------------------------------------------
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 #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#issuecomment-706775829
Thanks for the review working on 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 commented on a change in pull request #1290: Added and Enforced AbbreviationAsWordInName Checkstyle (FINERACT-821)
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r491842279
##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/LoanWithWaiveInterestAndWriteOffIntegrationTest.java
##########
@@ -140,7 +140,7 @@ public void checkClientLoanCreateAndDisburseFlow() {
}
@Test
- public void checkClientLoan_WRITTEN_OFF() {
+ public void checkClientLoanWrittenOFF() {
Review comment:
fixed.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/EmbeddedTomcatWithSSLConfiguration.java
##########
@@ -55,7 +55,7 @@ protected Connector createSslConnector() {
File keystore = getFile(getKeystore());
connector.setScheme("https");
connector.setSecure(true);
- connector.setPort(getHTTPSPort());
+ connector.setPort(getHTTPSport());
Review comment:
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