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.

Sign up for GitHub

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

Jump to bottom

fix: Time loses microseconds #9081

Merged
merged 15 commits into from
Jul 31, 2024
Merged

fix: Time loses microseconds #9081

merged 15 commits into from
Jul 31, 2024

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jul 26, 2024 •
edited
Loading

Description
Fixes #9079

Since PHP 7.1, DateTime constructor incorporates microseconds.
See https://www.php.net/manual/en/migration71.incompatible.php#migration71.incompatible.datetime-microseconds
and Time constructor also holds microseconds in most cases.
But some methods below in Time lost microseconds.

  • __construct() ... not all cases
  • createFromFormat()
  • createFromInstance()
  • toDateTime()
  • setTestNow()
  • equals()
  • sameAs()
  • isBefore()
  • isAfter()

Also fixes:

  • DatetimeCast::set()

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jul 26, 2024
@kenjis kenjis mentioned this pull request Jul 26, 2024
Open
@kenjis
Copy link
Member Author

kenjis commented Jul 26, 2024

This may break existing apps. So I will send it to 4.6.

@kenjis kenjis added the breaking change Pull requests that may break existing functionalities label Jul 26, 2024
@kenjis kenjis force-pushed the fix-Time-loses-microseconds branch from 06bbcca to 8fdd362 Compare July 26, 2024 21:31
@kenjis kenjis changed the base branch from develop to 4.6 July 26, 2024 21:31
@kenjis kenjis added the 4.6 label Jul 26, 2024
@kenjis kenjis force-pushed the fix-Time-loses-microseconds branch 2 times, most recently from 4e892ef to da64622 Compare July 27, 2024 02:49
@kenjis
Copy link
Member Author

kenjis commented Jul 27, 2024 •
edited
Loading

Added the docs, and fixed a bug in Time::toDateTime().
Perhaps there is still a bug that causes microseconds to be lost.

@kenjis kenjis force-pushed the fix-Time-loses-microseconds branch 2 times, most recently from 381f396 to 5a2a67c Compare July 27, 2024 04:14
@kenjis
Copy link
Member Author

kenjis commented Jul 27, 2024

Fixed isBefore() and isAfter().

@kenjis kenjis force-pushed the fix-Time-loses-microseconds branch from 5a2a67c to d438d05 Compare July 28, 2024 00:41
@kenjis kenjis force-pushed the fix-Time-loses-microseconds branch 2 times, most recently from c68a3fb to 56c6c46 Compare July 29, 2024 01:53
@kenjis
Copy link
Member Author

kenjis commented Jul 29, 2024

Fixed DatetimeCast::set() and added notes to the docs.

michalsn
michalsn approved these changes Jul 29, 2024
View reviewed changes
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

Looks great!

datamweb
datamweb approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@datamweb datamweb left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions bot added the stale Pull requests with conflicts label Jul 30, 2024
@github-actions GitHub Actions
Copy link

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@kenjis kenjis force-pushed the fix-Time-loses-microseconds branch from dea9d5b to ed29b50 Compare July 31, 2024 02:15
@kenjis kenjis removed the stale Pull requests with conflicts label Jul 31, 2024
@kenjis kenjis merged commit b3d957d into codeigniter4:4.6 Jul 31, 2024
42 checks passed
@kenjis kenjis deleted the fix-Time-loses-microseconds branch July 31, 2024 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datamweb datamweb datamweb approved these changes

@michalsn michalsn michalsn approved these changes

@paulbalandan paulbalandan Awaiting requested review from paulbalandan

Assignees
No one assigned
Labels
4.6 breaking change Pull requests that may break existing functionalities bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: Time loses microseconds
4 participants
@kenjis @michalsn @datamweb @paulbalandan