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

Change the "invalid" date to a "more invalid" date #6

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@HaraldJoerg
Contributor

HaraldJoerg commented Sep 4, 2018

That fix is a rather plain symtom fix, to make CPAN testers happy again. See below for my doubts about adding an extra test.

@heytrav

This comment has been minimized.

Show comment
Hide comment
@heytrav

heytrav Sep 4, 2018

I'm happy with this. One thing that might be good though is to add another test with this date that is expected to pass. The thinking here is just that if the original format is now technically valid a test should capture that as well.

heytrav commented on 584be43 Sep 4, 2018

I'm happy with this. One thing that might be good though is to add another test with this date that is expected to pass. The thinking here is just that if the original format is now technically valid a test should capture that as well.

This comment has been minimized.

Show comment
Hide comment
@HaraldJoerg

HaraldJoerg Sep 4, 2018

Owner

I am hesitating about that extra test. The timezone '+200' is invalid according to ISO8601, and it is only due to the current sloppy default parsing in DateTime::Format::Strptime that this format is "permitted" and interpreted as '+2000' (not '+0200'). Dave Rolsky has already announced (https://metacpan.org/pod/DateTime::Format::Strptime) that he might change the default in the future, which would make your tests break again at some random time.

Owner

HaraldJoerg replied Sep 4, 2018

I am hesitating about that extra test. The timezone '+200' is invalid according to ISO8601, and it is only due to the current sloppy default parsing in DateTime::Format::Strptime that this format is "permitted" and interpreted as '+2000' (not '+0200'). Dave Rolsky has already announced (https://metacpan.org/pod/DateTime::Format::Strptime) that he might change the default in the future, which would make your tests break again at some random time.

This comment has been minimized.

Show comment
Hide comment
@heytrav

heytrav Sep 4, 2018

Ah ok I might have misunderstood then. I understood that the the date is technically valid. If it is as you say then you're right that it wouldn't make sense

heytrav replied Sep 4, 2018

Ah ok I might have misunderstood then. I understood that the the date is technically valid. If it is as you say then you're right that it wouldn't make sense

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