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.

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

parse_interval support for full syntax #7

Merged
merged 6 commits into from Jun 19, 2015

Conversation

Projects
None yet
3 participants
@nbezzala
Contributor

nbezzala commented Feb 8, 2015

Solution for issue #5
Still need to add support for frc. I don't really understand what it is, how you are getting it and how that part works.

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Feb 8, 2015

Collaborator

The fact that pre-existing tests needed to be changed kind of bugs me. Does this mean it's going to break existing behavior?

Collaborator

lestrrat commented Feb 8, 2015

The fact that pre-existing tests needed to be changed kind of bugs me. Does this mean it's going to break existing behavior?

@ralesk

This comment has been minimized.

Show comment
Hide comment
@ralesk

ralesk Feb 18, 2015

Contributor

From what I can see the modified test line was actually buggy (1 day 1 month certainly doesn't mean -1 day), and the commented-out negative data are certainly supported forms by Postgres (9.3 in my case anyway).

If anyone built upon the wrong assumption that '1 day 1 month' has negative day, not sure how to act, their program is probably already broken, and a bug has been fixed. Certainly changes existing behaviour, but that behaviour was bad.

Looks like a nice patch though :)

@nbezzala Does your patch make the module support secondless time forms? Pg seems to be fine with select '12:30'::interval and select '1980-11-22 12:30'::timestamp but last I checked DateTime::Format::Pg croaked on the lack of seconds in the format.

Contributor

ralesk commented Feb 18, 2015

From what I can see the modified test line was actually buggy (1 day 1 month certainly doesn't mean -1 day), and the commented-out negative data are certainly supported forms by Postgres (9.3 in my case anyway).

If anyone built upon the wrong assumption that '1 day 1 month' has negative day, not sure how to act, their program is probably already broken, and a bug has been fixed. Certainly changes existing behaviour, but that behaviour was bad.

Looks like a nice patch though :)

@nbezzala Does your patch make the module support secondless time forms? Pg seems to be fine with select '12:30'::interval and select '1980-11-22 12:30'::timestamp but last I checked DateTime::Format::Pg croaked on the lack of seconds in the format.

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Feb 18, 2015

Collaborator

@ralesk @nbezzala just give me one more "Everything is OK", and I will do the rest.

Collaborator

lestrrat commented Feb 18, 2015

@ralesk @nbezzala just give me one more "Everything is OK", and I will do the rest.

@nbezzala

This comment has been minimized.

Show comment
Hide comment
@nbezzala

nbezzala Feb 20, 2015

Contributor

@ralesk Yes, it supports a duration like '12:30'. And I've added a couple of tests to check.
@lestrrat It looks goot to me.

Contributor

nbezzala commented Feb 20, 2015

@ralesk Yes, it supports a duration like '12:30'. And I've added a couple of tests to check.
@lestrrat It looks goot to me.

@ralesk

This comment has been minimized.

Show comment
Hide comment
@ralesk

ralesk Jun 19, 2015

Contributor

Anything up with this one? :)

Contributor

ralesk commented Jun 19, 2015

Anything up with this one? :)

lestrrat added a commit that referenced this pull request Jun 19, 2015

Merge pull request #7 from nbezzala/master
parse_interval support for full syntax

@lestrrat lestrrat merged commit 566694a into lestrrat-p5:master Jun 19, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

lestrrat added a commit that referenced this pull request Jun 19, 2015

Checking in changes prior to tagging of version 0.16011.
Changelog diff is:

diff --git a/Changes b/Changes
index 41cc962..28fe9cc 100644
--- a/Changes
+++ b/Changes
@@ -2,6 +2,9 @@ Revision history for Perl extension DateTime::Format::Pg.

 {{$NEXT}}

+0.16011 2015-06-19T13:40:27Z
+    - Support full interval syntax [pr #7]
+
 0.16010 2014-09-18T12:36:03Z
     - Add support for various more interval units [pr #3]:
       millenium, century, decade, month, week
@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Jun 19, 2015

Collaborator

hmm, I guess I didn't notice the last email sent to me.
Done and done

Collaborator

lestrrat commented Jun 19, 2015

hmm, I guess I didn't notice the last email sent to me.
Done and done

lestrrat added a commit that referenced this pull request Jul 19, 2016

Checking in changes prior to tagging of version 0.16012.
Changelog diff is:

diff --git a/Changes b/Changes
index 28fe9cc..64896d0 100644
--- a/Changes
+++ b/Changes
@@ -2,6 +2,11 @@ Revision history for Perl extension DateTime::Format::Pg.

 {{$NEXT}}

+0.16012 2016-07-19T21:37:31Z
+    - Parsing invalid intervals with no amount and units only should have
+      resulted in exceptions, but did not. Reported by Henrik Pauli (ssue #8)
+    - Internal cleanup
+
 0.16011 2015-06-19T13:40:27Z
     - Support full interval syntax [pr #7]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment