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/04/12 15:58:40 UTC
[GitHub] [fineract] percyashu opened a new pull request #755: FINERACT-826
Remove unused getTimeZone
percyashu opened a new pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
## Description
Describe the changes made and why they were made. Ignore if these details are present on the associated Jira ticket: https://issues.apache.org/jira/browse/FINERACT-826
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] All Integrations tests are passing with the new commits.
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-613894541
@vorburger will make 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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu opened a new pull request #755: FINERACT-826
Remove unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu opened a new pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
## Description
Describe the changes made and why they were made. Ignore if these details are present on the associated Jira ticket: https://issues.apache.org/jira/browse/FINERACT-826
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] All Integrations tests are passing with the new commits.
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger commented on a change in pull request #755:
FINERACT-826 Remove unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#discussion_r410713293
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java
##########
@@ -41,7 +41,7 @@ public static DateTimeZone getDateTimeZoneOfTenant() {
DateTimeZone zone = null;
if (tenant != null) {
zone = DateTimeZone.forID(tenant.getTimezoneId());
- TimeZone.getTimeZone(tenant.getTimezoneId());
+// TimeZone.getTimeZone(tenant.getTimezoneId());
Review comment:
@awasum this was done, so I'm going to merge this it - one less
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-612678291
> Why did you send this as a draft? @percyashu ?
I also want to check if very tenant is guaranteed to always have a TZ.
@vorburger seems it falls dependent on the OS / JVM TZ if tenant TZ is null.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu closed pull request #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu closed pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu opened a new pull request #755: FINERACT-826
Remove unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu opened a new pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
## Description
Describe the changes made and why they were made. Ignore if these details are present on the associated Jira ticket: https://issues.apache.org/jira/browse/FINERACT-826
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] All Integrations tests are passing with the new commits.
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] awasum commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
awasum commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-612641696
Why did you send this as a draft? @percyashu ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu closed pull request #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu closed pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu opened a new pull request #755: FINERACT-826
Remove unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu opened a new pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
## Description
Describe the changes made and why they were made. Ignore if these details are present on the associated Jira ticket: https://issues.apache.org/jira/browse/FINERACT-826
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] All Integrations tests are passing with the new commits.
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-613896690
@percyashu re. build failure, please subscribe to and watch (or help fix??) #764 and #761 for [FINERACT-852](https://issues.apache.org/jira/browse/FINERACT-852)... that's a mess, and help would be most welcome, see recent thread on the mailing list.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] awasum commented on a change in pull request #755:
FINERACT-826 Remove unused getTimeZone
Posted by GitBox <gi...@apache.org>.
awasum commented on a change in pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#discussion_r407221444
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java
##########
@@ -41,7 +41,7 @@ public static DateTimeZone getDateTimeZoneOfTenant() {
DateTimeZone zone = null;
if (tenant != null) {
zone = DateTimeZone.forID(tenant.getTimezoneId());
- TimeZone.getTimeZone(tenant.getTimezoneId());
+// TimeZone.getTimeZone(tenant.getTimezoneId());
Review comment:
Just completely remove the line...Dont comment it out.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger merged pull request #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger merged pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-615538908
see https://issues.apache.org/jira/browse/FINERACT-899 re. build failure...
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu opened a new pull request #755: FINERACT-826
Remove unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu opened a new pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
## Description
Describe the changes made and why they were made. Ignore if these details are present on the associated Jira ticket: https://issues.apache.org/jira/browse/FINERACT-826
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] All Integrations tests are passing with the new commits.
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-615403040
@percyashu following #764 and #761 this will pass the build (or not at least not fail because of integration tests) if you rebase this now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger commented on issue #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755#issuecomment-612828128
@percyashu I guess the IT failures here are unrelated to this change, see [FINERACT-885](https://jira.apache.org/jira/browse/FINERACT-885). Just keep trying once a day or so, until we have a green build? I know it's stupid.. if you can dig more into FINERACT-885, perhaps try fixing [FINERACT-723](https://jira.apache.org/jira/browse/FINERACT-723), that would be cool!
> I also want to check if very tenant is guaranteed to always have a TZ.
> @vorburger seems it falls dependent on the OS / JVM TZ if tenant TZ is null.
if it's not mandatory (`NON NULL`) in the DB, then you can't be sure, right? It would break backwards compatibility for any existing installations out there, agreed? The only thing we (you) could do if you want to fix this is to make a Flyway migration script that sets it to required? Then anyone upgrading would fail, and would be forced to set a TZ for each of their Tenants, if they don't already have one. Then you could make this code assume there always is one, and throw a (clear... include reference to a JIRA!) exception.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] percyashu closed pull request #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
percyashu closed pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [fineract] vorburger closed pull request #755: FINERACT-826 Remove
unused getTimeZone
Posted by GitBox <gi...@apache.org>.
vorburger closed pull request #755: FINERACT-826 Remove unused getTimeZone
URL: https://github.com/apache/fineract/pull/755
----------------------------------------------------------------
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
With regards,
Apache Git Services