some more tests for the "duration" format #402
Conversation
| }, | ||
| { | ||
| "description": "half a year duration, with full stop decimal point", | ||
| "comment": "ISO 8601 revision in year 2000 added decimals", |
There was a problem hiding this comment.
It doesn't seem like these apply, if I read the spec right. The spec says:
Date and time format names are derived from RFC 3339, section 5.6. The duration format is from the ISO 8601 ABNF as given in Appendix A of RFC 3339.
The ABNF there doesn't look like it allows these, no? The 2000 revision doesn't seem like it should apply.
There was a problem hiding this comment.
I think it is more useful to include the modern spec than one that is 25+ years old (I believe RFC3339 is intending to reference ISO8601 as a whole, rather than redefining it with restrictions), but this is not a hill I want to die on. I'd ask @handrews to update the spec to include the latest ISO8601 revisions, but I doubt he wants to spend another minute more thinking about formats.
There was a problem hiding this comment.
I don't necessarily disagree on the spec side, but the test suite isn't the place for innovation -- if the spec does mean to include this behavior then that's all fine and we should, but if not, we need to first update the spec, and then apply the behavior to the drafts it will (in the future) apply to.
There was a problem hiding this comment.
(To be clear, I think the action item here is "find out whether the spec intends to include this or not" -- and if it doesn't, we have to axe these, and then we can both add the clarification to the next spec and then at that point add these then)
But probably also easier to merge all the other indisputable tests first, so might be good to again split this PR so that those can be merged. (Or otherwise we can just keep this open until we clarify?)
There was a problem hiding this comment.
I created a ticket here -- json-schema-org/json-schema-spec#947 -- does this summarize the situation fully?
There was a problem hiding this comment.
If we split the PR in 2 some of these that aren't related to that ticket can get merged immediately if you want? Otherwise yeah we can just leave it until the spec addresses.
There was a problem hiding this comment.
agreed - pulled out into #412. I can rebase this PR later but will leave it alone for now so the conversation is not lost.
|
Yes looks great! Thanks!
…On Tue, Jun 16, 2020, 14:26 Karen Etheridge ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tests/draft2019-09/optional/format/duration.json
<#402 (comment)>
:
> + "data": "P1DT12H",
+ "valid": true
+ },
+ {
+ "description": "two weeks",
+ "data": "P2W",
+ "valid": true
+ },
+ {
+ "description": "weeks cannot be combined with other units",
+ "data": "P1Y2W",
+ "valid": false
+ },
+ {
+ "description": "half a year duration, with full stop decimal point",
+ "comment": "ISO 8601 revision in year 2000 added decimals",
I created a ticket here -- json-schema-org/json-schema-spec#947
<json-schema-org/json-schema-spec#947> -- does
this summarize the situation fully?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#402 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACQQXSRF7O3ZSUE32UQ65DRW62OFANCNFSM4N3VF35Q>
.
|
|
@karenetheridge it sounds like frustration aside that upstream we may not all be in agreement about improvements to formats, that json-schema-org/json-schema-spec#947 (comment) is saying "the existing specs mean that BNF and no more", i.e. to close and leave these out (or to put them in a second Do you agree? |
karenetheridge
added
the
on hold
|
@karenetheridge going to close this just to make ourselves look better (with 20 open PRs, oy...) -- I think we basically can't do anything here until/unless something changes in the spec. If that ever happens, obviously we should revisit. |
|
Could this perhaps be landed without the exact tests that raised concerns? |
|
Ah, sorry, that happened in #412. |
No description provided.