Skip to content

Conversation

deepakhalale
Copy link

No description provided.


@Test
public void dateTimeSecFracSuccess() {
assertSuccess("2015-02-28T11:00:00.111Z", new DateTimeFormatValidator());
Copy link
Contributor

Choose a reason for hiding this comment

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

@deepakhalale Is there any reason to remove this testcase?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @erosb, don't think I have removed the test case. I have renamed it to dateTimeWithThreeDigitsInSecFracSuccess and updated the seconds fraction from "111" to "123" to aligh with rest of the test cases that I added.
If you think the test case was there for a reason, then I will be glad to put it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, found it.
One final note: I've just found in the RFC3339 document that their ABNF defines fractions as:

time-fraction     = ("," / ".") 1*DIGIT

I had a quick glance at the javadoc and I couldn't find any easy way to support comma too, not only dot as a separator character. Do you think there is any simple solution to fix it?

Copy link
Author

Choose a reason for hiding this comment

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

The Json Schema Validation spec seem to suggest that the date-time format need to specifically conform to Section 5.6 of RFC-3339.
I think Section 5.6 of RFC-3339 is a specific cut down version of the ISO 8601 grammar and the ABNF date time that your referring to in the above comment, is an ISO 8601 standard.
Hope it helps.

@deepakhalale deepakhalale changed the title Implement variable length seconds fraction to confirm with RFC-3339 Implement variable length seconds fraction to conform with RFC-3339 Nov 28, 2016
@erosb
Copy link
Contributor

erosb commented Nov 28, 2016

All looks fine, merging. Thank you for your contribution @deepakhalale !

@erosb erosb merged commit 13c2162 into everit-org:master Nov 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants