Fix for #21531
Comment From: pivotal-issuemaster
@gaurav-91 Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.
Comment From: pivotal-issuemaster
@gaurav-91 Thank you for signing the Contributor License Agreement!
Comment From: gaurav-91
@philwebb: Did you get a chance to go through the PR. Please let me know if code need to be reviewed together. thanks
Comment From: philwebb
@gaurav-91 I'm afraid we have a number of bugs that need my attention before we release 2.3.1. It might be another week or so before I can get to this.
Comment From: mbhave
@gaurav-91 Thanks for the PR. Could you add some tests to make sure that the iso-offset pattern works as expected. You can take a look at the existing test for the "iso" pattern.
Comment From: gaurav-91
Sure will add it by today.
Comment From: gaurav-91
@mbhave I have added the test cases but they seem to be failing. Looks like OffsetDateTime converters are being used. And if I use LocalDate Object that fails while parsing as we are trying to parse ISO_Offset date. Could you please take a look what I am doing wrong here. Thanks
Comment From: gaurav-91
@mbhave Could you please take a look at the test cases added by me, and help me understand what I am doing wrong in that. Thanks
Comment From: wilkinsona
Thanks for the PR, @gaurav-91.
I don't think it makes sense to offer ISO offset formatting for dates. The date format is used to configure the date format of Spring Framework's DateTimeFormatterRegistrar and the javadoc of its setDateFormatter method states that it is only used for formatting of LocalDate. A LocalDate does not contain a timezone offset so it cannot be formatted using an offset-based format as there's no offset information to format. Would you mind reworking your proposal slightly so that iso-offset is only supported for the spring.mvc.format.date-time and spring.mvc.format.time properties?
Comment From: gaurav-91
Thank you @wilkinsona . It syncs up with my understanding, I will start working on the changes mentioned by you.
Comment From: wilkinsona
How's it going, @gaurav-91?
Comment From: gaurav-91
@wilkinsona I have made the changes as suggested by you. Test case for spring.mvc.format.time seems to be failing. I have committed the code done so far. Could you please take look and advice if possible. Thanks
Comment From: wilkinsona
Thanks very much for making your first contribution to Spring Boot, @gaurav-91. The proposed changes have now been merged into master. If you're interested, I also added a small polishing commit, primarily to allow iso-offset to be used as the value as well as isooffset.
Comment From: gaurav-91
@wilkinsona Any other issues which I could pick up.
Comment From: wilkinsona
@gaurav-91 Thanks for the offer. We have an ideal for contribution label that may be of interest.