Skip to content
This repository has been archived by the owner on Apr 2, 2019. It is now read-only.

Relax type hints to DateTimeInterface and make use of DateTimeImmutable #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gws
Copy link
Owner

@gws gws commented Feb 24, 2017

This change breaks backward compatibility, in that getStart() and getEnd() will always return instances of DateTimeImmutable instead of DateTime as they did previously.

PHP >=5.6 is now required. PHP 5.5 is EOL, and users who cannot upgrade should use older releases of this library.

This change breaks backward compatibility, in that getStart() and
getEnd() will always return instances of DateTimeImmutable instead of
DateTime as they did previously.

PHP >=5.6 is now required. PHP 5.5 is EOL, and users who cannot upgrade
should use older releases of this library.
@rquadling
Copy link
Contributor

I've got some commits on this. Includes coverage to close to 100%. Also upgraded to PHPUnit 6.

PHPStan and php-cs.

I'll create a PR when I get back to work.

@rquadling
Copy link
Contributor

I'd go with a minor release due to the BC break.

Also considering PHP5.6 is now locked for security fixes only, move to PHP 7+ only.

@@ -64,14 +64,14 @@ public function diff(DateRange $arg)

if ($this->getStart() < $arg->getStart()) {
return new static(
clone $this->getStart(),
date_modify(clone $arg->getStart(), '-1 second')
$this->getStart(),
Copy link
Contributor

Choose a reason for hiding this comment

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

This one caught me when I did this. Can't use date_modify() on immutables.

@@ -3,6 +3,7 @@
namespace VoTest;

use DateTime;
use DateTimeImmutable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Mutable dates still need to be tested as it is the VO's job to make them immutable.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Immutable dates also need to be tested. There's a mix of mutable and immutable if you look at the tests, though it should be sufficient to test that the constructor, separately, provides an immutable date invariant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Yes. I missed that.

@rquadling
Copy link
Contributor

Would you consider PHP7+.

As 5.6 is now feature frozen (http://php.net/supported-versions.php).

@rquadling
Copy link
Contributor

If so, we could pull in PHPUnit 6 and PHPStan - both PHP7 only.

@gws
Copy link
Owner Author

gws commented Mar 6, 2017

Since we're making a breaking change requiring a major version release anyway, I'm fine with that. I'll make the changes soon.

Numerous fixes to accommodate changes in PHPUnit as well as some fixes
found by PHPStan.
@gws
Copy link
Owner Author

gws commented Mar 6, 2017

I believe I've made the necessary changes to pull in the new versions and projects you wanted.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants