Skip to content

feat: add Time between(), min(), and max() methods#10149

Open
michalsn wants to merge 5 commits intocodeigniter4:4.8from
michalsn:feat/time-between-min-max
Open

feat: add Time between(), min(), and max() methods#10149
michalsn wants to merge 5 commits intocodeigniter4:4.8from
michalsn:feat/time-between-min-max

Conversation

@michalsn
Copy link
Copy Markdown
Member

Description
This PR adds between(), min(), and max() methods to the TimeTrait.

Closes #10143

Checklist:

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

@michalsn michalsn added enhancement PRs that improve existing functionalities 4.8 PRs that target the `4.8` branch. labels Apr 30, 2026
Comment thread system/I18n/TimeTrait.php
Co-authored-by: neznaika0 <ozornick.ks@gmail.com>
Comment thread system/I18n/TimeTrait.php Outdated
*
* If $start is after $end, the arguments are swapped.
*
* @param DateTimeInterface|self|string $start
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Time and TimeLegacy extends from DateTimeImmutable and DateTime which both implement the interface, so phpstan would resolve these phpdocs as simply DateTimeInterface|string. Can we update both and make them native param types?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TimeLegacy is deprecated since 4.3.0. Perhaps it can be removed in 4.8.0

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

As for TimeLegacy - I'm curious to hear what others think. Should we do this now, or is it something that should wait until v5?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There’s a good point that has been raised here. I think it would be better if we could do this in v5.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is something I am asking myself. There's no explicit docs saying what to do for deprecated classes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In theory, this is subject to depreciation rules, but I would be cautious in the entire class case.

Comment thread system/I18n/TimeTrait.php Outdated
Comment thread system/I18n/TimeTrait.php Outdated
Comment thread system/I18n/TimeTrait.php Outdated
Comment thread system/I18n/TimeTrait.php Outdated
michalsn and others added 2 commits May 2, 2026 06:17
Co-authored-by: John Paul E. Balandan, CPA <paulbalandan@gmail.com>
Copy link
Copy Markdown
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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 PRs that target the `4.8` branch. enhancement PRs that improve existing functionalities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants