-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Adding ability to create a DateTime or DateTimeImmutable from an inst… #5016
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
Adding ability to create a DateTime or DateTimeImmutable from an inst… #5016
Conversation
+1 on these methods. Ideally we'd have just added these instead of createFromMutable+createFromImmutable. |
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.
I'm OK with this idea, as Mike and I discussed this recently.
@mikeSimonson |
fbcad83
to
fea4392
Compare
fea4392
to
2bbf3d4
Compare
As far as I can tell this PR is ready tested and respect the CS ? |
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.
Looks good to me, but let's wait until Derick has had a look at this as well.
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.
LGTM, but please fix the two nits before merging.
int(1) | ||
["timezone"]=> | ||
string(6) "+01:00" | ||
} |
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.
Nit: please make sure the last line also has a new line at the end (but not an extra empty line)
int(1) | ||
["timezone"]=> | ||
string(6) "+01:00" | ||
} |
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.
Nit: please make sure the last line also has a new line at the end (but not an extra empty line)
It seems to me that this PR obsoletes PR #3394. |
…ance of the interface