Skip to content
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

Fix casting to string for ChronosDate and ChronosTime. #428

Merged
merged 2 commits into from
Sep 24, 2023

Conversation

ADmad
Copy link
Member

@ADmad ADmad commented Sep 24, 2023

Make all classes explicitly implemement Stringable as recommeneded by the PHP docs.

Make all classes explicitly implemement Stringable as recommeneded
by the PHP docs.
@@ -32,7 +32,7 @@ trait FormattingTrait
*/
public static function resetToStringFormat(): void
{
static::setToStringFormat(Chronos::DEFAULT_TO_STRING_FORMAT);
static::setToStringFormat(static::DEFAULT_TO_STRING_FORMAT);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this fix and addition of the constant to ChronosDate, calling ChronosDate::__toString() after ChronosDate::resetToStringFormat() returned value in Y-m-d H:i:s format instead of H:i:s.

*
* @return void
*/
public static function resetToStringFormat(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably never needed the reset helpers since you can pass the constant to set directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just made it consistent with the other 2 classes.

@othercorey othercorey added this to the 3.x milestone Sep 24, 2023
@othercorey othercorey merged commit 7ffdb16 into 3.x Sep 24, 2023
7 checks passed
@othercorey othercorey deleted the 3.x-stringable branch September 24, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants