Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Allow DateTimeImmutable instances to be provided as entry, deletion and feed dates #93

Merged
merged 3 commits into from Jan 29, 2019
Merged

Conversation

gsteel
Copy link
Contributor

@gsteel gsteel commented Nov 8, 2018

The writers throw an exception if they receive a DateTimeImmutable instance, for example to WriterAbstract::setDateCreated()

Provide a narrative description of what you are trying to accomplish:

  • Are you fixing a bug?
    • Detail how the bug is invoked currently.
$myDate = new DateTimeImmutable('@' . 1234567890);
$writer = new Writer\Feed;
$writer->setDateModified($myDate);
  • Detail the original, incorrect behaviour.
uncaught Zend\Feed\Exception\InvalidArgumentException('Invalid DateTime object or UNIX Timestamp  passed as parameter')
  • Detail the new, expected behaviour.

General happiness

  • Base your feature on the master branch, and submit against that branch.
  • Add a regression test that demonstrates the bug, and proves the fix.
  • Add a CHANGELOG.md entry for the fix.

@@ -180,6 +181,14 @@ public function testSetDateCreatedUsesDateTimeObject()
$this->assertEquals($myDate, $writer->getDateCreated());
}

public function testSetDateCreatedUsesDateTimeImmutableObject()
{
$myDate = new DateTimeImmutable('@' . 1234567890);
Copy link
Member

Choose a reason for hiding this comment

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

We should definitely use data-providers for the entire test class – later!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hell yeah!

@froschdesign
Copy link
Member

@gsteel
It would be great if you could also update the documentation. Please have a look at: https://github.com/zendframework/zend-feed/blob/master/docs/book/writer.md#feed-api-methods

Thanks in advance!

@froschdesign froschdesign self-assigned this Nov 8, 2018
@froschdesign froschdesign added this to the 2.10.0 milestone Nov 8, 2018
@froschdesign
Copy link
Member

@gsteel
Please use the develop branch as your changes are an improvement and not a bug fix. Thanks!

@gsteel gsteel changed the base branch from master to develop November 9, 2018 11:20
@weierophinney weierophinney merged commit 6c47bab into zendframework:develop Jan 29, 2019
weierophinney added a commit that referenced this pull request Jan 29, 2019
Allow DateTimeImmutable instances to be provided as entry, deletion and feed dates

Conflicts:
	CHANGELOG.md
weierophinney added a commit that referenced this pull request Jan 29, 2019
@weierophinney
Copy link
Member

Thanks, @gsteel!

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

Successfully merging this pull request may close these issues.

None yet

3 participants