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/01 06:13:27 UTC
[GitHub] [fineract] percyashu opened a new pull request #1131: FINERACT-822 Enforcing Checks
percyashu opened a new pull request #1131:
URL: https://github.com/apache/fineract/pull/1131
FINERACT-822
Enforce StaticAssignmentInConstructor check
Enable ProtectedMembersInFinalClass check
----------------------------------------------------------------
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 #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
vorburger merged pull request #1131:
URL: https://github.com/apache/fineract/pull/1131
----------------------------------------------------------------
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] percyashu commented on a change in pull request #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
percyashu commented on a change in pull request #1131:
URL: https://github.com/apache/fineract/pull/1131#discussion_r448686313
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/domain/Report.java
##########
@@ -111,7 +111,7 @@ public static Report fromJson(final JsonCommand command, final Collection<String
return new Report(reportName, reportType, reportSubType, reportCategory, description, useReport, reportSql, reportTypes);
}
- protected Report() {
+ Report() {
Review comment:
I look at these and JPA requires entities to have a no-args constructo .
----------------------------------------------------------------
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] percyashu commented on a change in pull request #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
percyashu commented on a change in pull request #1131:
URL: https://github.com/apache/fineract/pull/1131#discussion_r448275805
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/domain/Report.java
##########
@@ -111,7 +111,7 @@ public static Report fromJson(final JsonCommand command, final Collection<String
return new Report(reportName, reportType, reportSubType, reportCategory, description, useReport, reportSql, reportTypes);
}
- protected Report() {
+ Report() {
Review comment:
Sure will remove them
----------------------------------------------------------------
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] awasum commented on pull request #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #1131:
URL: https://github.com/apache/fineract/pull/1131#issuecomment-653529555
/rebase
----------------------------------------------------------------
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 #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1131:
URL: https://github.com/apache/fineract/pull/1131#discussion_r448240409
##########
File path: fineract-provider/src/test/java/org/apache/fineract/common/RestAssuredFixture.java
##########
@@ -33,16 +32,14 @@
*/
public class RestAssuredFixture {
- private final int httpPort;
+ private final int httpPort = 8443;
private ResponseSpecification responseSpec;
private RequestSpecification requestSpec;
- public RestAssuredFixture(int httpPort) {
+ public RestAssuredFixture() {
super();
- this.httpPort = httpPort;
Utils.initializeRESTAssured();
- RestAssured.port = httpPort;
Review comment:
I was quite surprised that you were able to remove this - how come? Is RestAssured's default 8443? Isn't it better to keep this explicit?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/domain/Report.java
##########
@@ -111,7 +111,7 @@ public static Report fromJson(final JsonCommand command, final Collection<String
return new Report(reportName, reportType, reportSubType, reportCategory, description, useReport, reportSql, reportTypes);
}
- protected Report() {
+ Report() {
Review comment:
@percyashu couldn't this just be removed, instead of made package local?
----------------------------------------------------------------
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] percyashu closed pull request #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
percyashu closed pull request #1131:
URL: https://github.com/apache/fineract/pull/1131
----------------------------------------------------------------
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] percyashu commented on a change in pull request #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
percyashu commented on a change in pull request #1131:
URL: https://github.com/apache/fineract/pull/1131#discussion_r448686741
##########
File path: fineract-provider/src/test/java/org/apache/fineract/common/RestAssuredFixture.java
##########
@@ -33,16 +32,14 @@
*/
public class RestAssuredFixture {
- private final int httpPort;
+ private final int httpPort = 8443;
private ResponseSpecification responseSpec;
private RequestSpecification requestSpec;
- public RestAssuredFixture(int httpPort) {
+ public RestAssuredFixture() {
super();
- this.httpPort = httpPort;
Utils.initializeRESTAssured();
- RestAssured.port = httpPort;
Review comment:
Does this makes it more explicit
----------------------------------------------------------------
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] percyashu commented on a change in pull request #1131: FINERACT-822 Enforcing Checks
Posted by GitBox <gi...@apache.org>.
percyashu commented on a change in pull request #1131:
URL: https://github.com/apache/fineract/pull/1131#discussion_r448274655
##########
File path: fineract-provider/src/test/java/org/apache/fineract/common/RestAssuredFixture.java
##########
@@ -33,16 +32,14 @@
*/
public class RestAssuredFixture {
- private final int httpPort;
+ private final int httpPort = 8443;
private ResponseSpecification responseSpec;
private RequestSpecification requestSpec;
- public RestAssuredFixture(int httpPort) {
+ public RestAssuredFixture() {
super();
- this.httpPort = httpPort;
Utils.initializeRESTAssured();
- RestAssured.port = httpPort;
Review comment:
RestAssured.port is a static field hence picked up by StaticAssignmentInConstructor check.
I removed that line because Utils.initializeRESTAssured() sets the port to 8443.
I could try creating a method in Utils to initialize RestAssured with specific port or just revert this leave the check disabled.
----------------------------------------------------------------
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