-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: make Time::__toString() database-compatible on any locale #6461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I think this is the way to go. Since that format is very clearly "database DateTime" I expect the intent was always to have the Stringable be database-compatible. |
It always returns string like '2016-03-09 12:00:00'.
0bb6483
to
3e2e401
Compare
setUp()/tearDown() take care of it.
dbf7348
to
32e6c9d
Compare
Co-authored-by: Pooya Parsa Dadashi <[email protected]>
Added the docs. |
Co-authored-by: John Paul E. Balandan, CPA <[email protected]>
Co-authored-by: John Paul E. Balandan, CPA <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kenjis, I know you have sent several PRs to solve this problem and finally here we are.
It works fine now.
Thanks.
Thank you all! |
🎉 |
Description
Closes #6439
Supersedes #6456
In this PR,
Time::__toString()
no longer depends on locale.It always returns database-compatible string like
'2016-03-09 12:00:00'
.In the current implementation, the
__toString()
output will be changed by the locale.(But it seems it is not documented in the User Guide.)
It causes database query error in a few locale.
Time is localized class, but
__toString()
does not need to return localized value.If a dev needs localized string, they can call toDateTimeString().
Implicitly changing behavior depending on locale is a bad practice because it confuses developers.
__toString()
is a standard PHP method, not a localized helper method.The return value should be database-compatible.
Checklist: