Skip to content
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

Updated date-time regex to accept zero or one of : in the timezone designator #85

Merged
merged 7 commits into from
Dec 31, 2013

Conversation

jwarykowski
Copy link
Contributor

Hi Kenny,

I've updated the date-time regex to accept zero or one of ":" in the timezone. I've noticed that the ISO-8601 standard states (http://www.w3.org/TR/NOTE-datetime) that there should always be a colon but have found that in other sources that sometimes the colon doesn't appear. Please see the sources below:

  1. http://en.wikipedia.org/wiki/ISO_8601 - see time zone designators section
  2. http://www.cl.cam.ac.uk/~mgk25/iso-time.html - see timezone section

I've added a few tests within test_jsonschema_draft4.rb and removed a test which no longer fails in test_jsonschema_draft3.rb.

Let me know if you want anything else added? If you don't want to merge this PR for any reason no worries, if you could let me know why (just out of interest) that would be great.

Regards,
Jon

@apsoto
Copy link
Contributor

apsoto commented Oct 31, 2013

Typically you should avoid editing version files since when the release is made is up to the project's committers. I'd say make a new PR without that change to the version file.

@jwarykowski
Copy link
Contributor Author

Reverted the commit.

@apsoto
Copy link
Contributor

apsoto commented Oct 31, 2013

hopefully @hoxworth has some time soon to run through a few of the pending PR's

@jwarykowski
Copy link
Contributor Author

I've just noticed I've made a massive error on the tests.

assert(!JSON::Validator.validate(schema,data))

The tests are returning false and then getting flipped to true. I'll try and fix this problem and update the PR.

Regards,
Jon

@jwarykowski
Copy link
Contributor Author

Added two new commits which now rectify the issues I had in my previous comment, it turns out the tests had an invalid iso-8601 date-time (never try and rush a PR on your lunch break).

Cheers,
Jon

hoxworth added a commit that referenced this pull request Dec 31, 2013
Updated date-time regex to accept zero or one of : in the timezone designator
@hoxworth hoxworth merged commit 5d00dc4 into voxpupuli:master Dec 31, 2013
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.

3 participants