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

Fix for #3374 #3375

Merged
merged 8 commits into from
Jan 9, 2013
Merged

Fix for #3374 #3375

merged 8 commits into from
Jan 9, 2013

Conversation

dkidd
Copy link
Contributor

@dkidd dkidd commented Jan 7, 2013

DateTime::ISO8601 doesn't properly handle fractions of a second. This patch fixes that problem.

DateTime::ISO8601 doesn't properly handle fractions of a second. This patch fixes that problem.
$format = 'Y-m-d\TH:i:s.uO';
} else {
$format = DateTime::ISO8601;
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd change the logic to this:

$format = DateTime::ISO8601;
if ($dateCreated[19] === '.') {
    $format = 'Y-m-d\TH:i:s.uO';
}

This eliminates the else condition, which is the default and expected condition.

@weierophinney
Copy link
Member

@dkidd Could you add a test for this, by any chance?

Added logic to detect and properly handle ISO8601 date strings that contain fractions of a second.
@dkidd
Copy link
Contributor Author

dkidd commented Jan 7, 2013

@weierophinney I'd be happy to write tests for getDateCreated() and getDateModified().

@@ -203,7 +207,11 @@ public function getDateModified()
}

if ($dateModified) {
$date = DateTime::createFromFormat(DateTime::ISO8601, $dateModified);
$format = DateTime::ISO8601;
if ($dateCreated[19] === '.') {
Copy link
Contributor

Choose a reason for hiding this comment

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

should be dateModified :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eep!

@dkidd
Copy link
Contributor Author

dkidd commented Jan 8, 2013

@weierophinney See my latest commit for a proposed solution to this issue. I figured that, since Zend\Feed\Reader\Entry\Atom isn't the only class affected, adding Zend\Stdlib\DateTime (extending \DateTime) would be a more elegant solution.

@dkidd
Copy link
Contributor Author

dkidd commented Jan 9, 2013

Does anyone know why the Travis build keeps failing?

@tca3
Copy link
Contributor

tca3 commented Jan 9, 2013

@dkidd (trailing_spaces, braces, eof_ending) run your code through php-cs-fixer

@@ -0,0 +1 @@
Copy link
Member

Choose a reason for hiding this comment

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

This file should not be included in the Git history

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Very annoying. I'm not sure of the best way to remove it, though. Suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind. Figured it out. Thanks for the suggestion.

@Maks3w Maks3w merged commit 25b8d87 into zendframework:master Jan 9, 2013
@Maks3w
Copy link
Member

Maks3w commented Jan 9, 2013

Thank you

gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
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.

4 participants