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 2022/01/02 21:14:33 UTC
[GitHub] [fineract] vidakovic opened a new pull request #2013: Upgrade to JDK 17
vidakovic opened a new pull request #2013:
URL: https://github.com/apache/fineract/pull/2013
## Description
Upgrade to JDK 17
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic commented on a change in pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
vidakovic commented on a change in pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#discussion_r777460966
##########
File path: integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementDetailsIntegrationTest.java
##########
@@ -76,7 +76,7 @@ public void createAndValidateMultiDisburseLoansBasedOnEmi() {
String withoutInstallmentAmount = "";
String proposedAmount = "10000";
createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 June 2015", "5000"));
- createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 Sep 2015", "5000"));
+ createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 September 2015", "5000"));
Review comment:
Yes, that's exactly what was the problem here. I additionally verified by doing the reverse (creating a `LocalDate` on `2015-09-15` and print it out on the console with pattern `dd MMMM yyyy` and it prints out exactly above string. Curious, but seems like someone tightened the screws in JDK17. I think it actually makes sense, `Sep` is an abbreviation vs. `September` is the full month name (which is exactly the same behavior for the other months.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic edited a comment on pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
vidakovic edited a comment on pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#issuecomment-1003855638
@ptuomola ... looks like a lot of changes, but in the end it boils down to one missing GSON JsonSerializer (added `org/apache/fineract/infrastructure/core/api/LocalTimeAdapter.java`) that is needed to make things work with JDK17. There is a new static method `GoogleGsonSerializerHelper.createGsonBuilder()` that should be used to instantiate `GsonBuilder` instead of using `new GsonBuilder()` ... well, the method was there before, but not static and unused... I guess it was just forgotten.
Once this one is approved and merged then I can finish up the Testcontainer integration (and Tomcat Cargo plugin removal).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#discussion_r777456266
##########
File path: integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementDetailsIntegrationTest.java
##########
@@ -76,7 +76,7 @@ public void createAndValidateMultiDisburseLoansBasedOnEmi() {
String withoutInstallmentAmount = "";
String proposedAmount = "10000";
createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 June 2015", "5000"));
- createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 Sep 2015", "5000"));
+ createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 September 2015", "5000"));
Review comment:
Does this change something that means the short format "01 Sep 2015" no longer works? I wasn't able to see any changes which would have that effect... so was a bit surprised to see this change.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] akki-pesit edited a comment on pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
akki-pesit edited a comment on pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#issuecomment-1005853043
Seems like after this change the swagger-ui is not able to find fineract.yaml. I might be wrong about the cause but we are getting the below error.
<img width="1792" alt="Screenshot 2022-01-05 at 9 28 08 PM" src="https://user-images.githubusercontent.com/18091801/148248421-72dfadf5-1ab0-4110-90b2-d5db5edb1575.png">
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic commented on pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#issuecomment-1003855638
@ptuomola ... looks like a lot of changes, but in the end it boils down to one missing GSON JsonSerializer (added `org/apache/fineract/infrastructure/core/api/LocalTimeAdapter.java`) that is needed to make things work with JDK17. There is a new static method `GoogleGsonSerializerHelper.createGsonBuilder()` that should be used to instantiate `GsonBuilder` instead of using `new GsonBuilder()` ... well, the method was there before, but not static and unused... I guess it was just forgotten.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#discussion_r777464762
##########
File path: integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementDetailsIntegrationTest.java
##########
@@ -76,7 +76,7 @@ public void createAndValidateMultiDisburseLoansBasedOnEmi() {
String withoutInstallmentAmount = "";
String proposedAmount = "10000";
createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 June 2015", "5000"));
- createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 Sep 2015", "5000"));
+ createTranches.add(this.loanTransactionHelper.createTrancheDetail(id, "01 September 2015", "5000"));
Review comment:
Ok - looks like this has indeed changed for JDK17: https://stackoverflow.com/questions/69267710/septembers-short-form-sep-no-longer-parses-in-java-17-in-en-gb-locale
It's not nice that this would potentially break some clients. But I suppose this should never happen as long as people are using machine generated dates - as they should be providing the correct string anyway. It's only if someone is manually typing "17 Sep" or hardcoding such strings.
But I don't think it warrants implementing some specific handling in Fineract, given that this will be the standard for all solutions compiled with JDK17.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic commented on pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#issuecomment-1005883486
@akki-pesit thanks for reporting... it's most likely unrelated to the JDK 17 upgrade. Please see https://issues.apache.org/jira/projects/FINERACT/issues/FINERACT-1480. Petri is taking care of it today.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] akki-pesit commented on pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
akki-pesit commented on pull request #2013:
URL: https://github.com/apache/fineract/pull/2013#issuecomment-1005853043
Seems like after this change the swagger-ui is not able to find fineract.yaml
<img width="1792" alt="Screenshot 2022-01-05 at 9 28 08 PM" src="https://user-images.githubusercontent.com/18091801/148248421-72dfadf5-1ab0-4110-90b2-d5db5edb1575.png">
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola merged pull request #2013: Upgrade to JDK 17
Posted by GitBox <gi...@apache.org>.
ptuomola merged pull request #2013:
URL: https://github.com/apache/fineract/pull/2013
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org