Closed agitrubard closed 5 months ago
aaberg
commented
6 months ago Hi @agitrubard, Thanks for submitting these converters, which I agree are missing in Sql2o. Can I persuade you to write some tests that verify that they work as expected? When that is done, I'll be happy to merge your contribution.
agitrubard
commented
6 months ago Hi @agitrubard, Thanks for submitting these converters, which I agree are missing in Sql2o. Can I persuade you to write some tests that verify that they work as expected? When that is done, I'll be happy to merge your contribution.
@aaberg Thank you for reviewing my contribution and taking the time to assess my pull request. Following your feedback, I have added the missing tests and resolved the conflict. I hope everything now aligns with your expectations as you review the changes. You can also verify the conflict resolution adjustments I made. If there's any further refinement required or if you have additional feedback, please feel free to let me know.
Thank you again for your time and consideration. Looking forward to your positive feedback!
aaberg
commented
5 months ago As this PR correctly states, java.time integration is essential since java.time classes are recommended for date and time representation. I'm closing this PR, since I'm not convinced it solves the integration with a robust enough solution, and the tests aren't good enough.
Take a look at this alternative implementation that will get merged instead.
https://github.com/aaberg/sql2o/pull/367
Description:
This pull request introduces a converter to accurately extract "java.time.LocalDateTime" instances from the database and map them correctly, extending support for the "java.time.LocalDate" class as well.
Why:
Java 8 and Above Date and Time API: This change supports the modern Java applications widely utilizing the Date and Time API introduced in Java 8. It particularly focuses on robustly representing local date and time information through the "LocalDateTime" and "LocalDate" classes.
Issues with the Date Class: Opting for "LocalDateTime" and "LocalDate" over the "java.util.Date" class promotes safer, more readable, and error-tolerant code. The "java.util.Date" class is discouraged in modern applications due to its inadequate design and various issues related to time.
Best Practice and Performance Gain: Using "LocalDateTime" and "LocalDate" aligns with better best practices for date and time manipulation. Additionally, this change may contribute to achieving more accurate results and performance improvements in time-related operations.
Community Demands: Observations indicate a growing demand within the community for the integration of "java.time.LocalDateTime" and "java.time.LocalDate" classes into the sql2o library.
Changes:
Acceptance of Your Contributions:
This change is made to align with the requirements of modern Java applications and enhance code quality. We welcome your feedback and any further corrections related to this matter.