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

fix datetime format for RFC3339 compliance #82

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

MarcoKorinth
Copy link

@MarcoKorinth MarcoKorinth commented May 17, 2023

Fixes #55
Does work for fast mode and full mode.

Fix tested with this example:
ajv.validate({ type: 'string', format: 'date-time' }, '2020-01-01 20:00:00.000Z')

@tschmidtb51 tschmidtb51 mentioned this pull request Jul 11, 2023
Copy link
Contributor

@tschmidtb51 tschmidtb51 left a comment

Choose a reason for hiding this comment

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

I still found some cases not covert yet...

src/formats.ts Outdated Show resolved Hide resolved
tests/index.spec.ts Outdated Show resolved Hide resolved
tests/index.spec.ts Show resolved Hide resolved
Copy link
Contributor

@tschmidtb51 tschmidtb51 left a comment

Choose a reason for hiding this comment

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

One minor comment - other than that: LGTM

tests/index.spec.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@tschmidtb51 tschmidtb51 left a comment

Choose a reason for hiding this comment

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

Please recheck that the line 200 change does not break the iso-date-time.

src/formats.ts Show resolved Hide resolved
MarcoKorinth and others added 2 commits July 13, 2023 23:04
Co-authored-by: tschmidtb51 <65305130+tschmidtb51@users.noreply.github.com>
Copy link
Contributor

@tschmidtb51 tschmidtb51 left a comment

Choose a reason for hiding this comment

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

I found a duplicate and a mistake in the testcases.

tests/extras/format.json Outdated Show resolved Hide resolved
tests/extras/format.json Outdated Show resolved Hide resolved
MarcoKorinth and others added 2 commits July 15, 2023 05:33
Co-authored-by: tschmidtb51 <65305130+tschmidtb51@users.noreply.github.com>
Co-authored-by: tschmidtb51 <65305130+tschmidtb51@users.noreply.github.com>
Copy link
Contributor

@tschmidtb51 tschmidtb51 left a comment

Choose a reason for hiding this comment

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

LGTM

@tschmidtb51
Copy link
Contributor

@epoberezkin, @ChALkeR Please review

@tschmidtb51
Copy link
Contributor

@ChALkeR , @epoberezkin Please review the suggested changes

@tschmidtb51
Copy link
Contributor

@jasoniangreen Could you please have a look an review this?

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

Successfully merging this pull request may close these issues.

date-time accepts timestamps without "T" separator between date and time
2 participants