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

Built-in format checker changes #78

Closed
gazpachoking opened this issue Mar 3, 2013 · 10 comments
Closed

Built-in format checker changes #78

gazpachoking opened this issue Mar 3, 2013 · 10 comments
Labels
Enhancement Some new desired functionality

Comments

@gazpachoking
Copy link
Contributor

It would be nice if all of the built in format checkers were available in the FormatChecker class, to ease the creation of an instance with a custom subset of any of the format checkers available.

We currently provide draft3_ and draft4_format_checker instances, which have the subset of checkers defined in each draft. These are a bit problematic, as it might be unexpected when format checkers defined in the spec are not available in them due to required libraries being present. Not sure what the best solution is yet.

@Julian
Copy link
Member

Julian commented Apr 11, 2013

Not having format checkers present when the required dependencies aren't satisfied doesn't bother me that much. If anything we could improve the documentation if you think there's something there that could be clearer. We could perhaps more prominently discuss the attribute on format checkers that contains all the known checkers, which is public and could be used to inspect the available checkers.

Putting all the format checkers in the class though is fine with me if we have a reasonable way of doing that.

Since this is mostly for our own sanity this is pretty low priority I'm guessing.

@gazpachoking
Copy link
Contributor Author

Since this is mostly for our own sanity this is pretty low priority I'm guessing.

My goal was so that an user can instantiate a FormatChecker with the subset of properties they desire. Right now you can't do FormatChecker(formats=('time', 'email')) because 'time' checker is just in the draft3_format_checker instance, not on the class.
I'll have another go at doing this.

@gazpachoking
Copy link
Contributor Author

What do you think about something like this? gazpachoking@b0a694f

@gazpachoking
Copy link
Contributor Author

Or maybe _checks_drafts should be usable like @_checks_drafts("email", raises=blah) for checkers on both drafts, and @_checks_drafts(draft3="ip-address", draft4="ipv4", raises=blah) for checkers on draft 3 only or when the name differs.

@Julian
Copy link
Member

Julian commented Apr 11, 2013

Um, I'm OK with that.

@gazpachoking
Copy link
Contributor Author

You sound hesitant...

@Julian
Copy link
Member

Julian commented Apr 11, 2013

:) nah. We should document the canonical names for the format checkers.
Other than that I'm not hesitant, just not particularly excited :p.
On Apr 11, 2013 9:08 AM, "Chase Sterling" notifications@github.com wrote:

You sound hesitant...


Reply to this email directly or view it on GitHubhttps://github.com//issues/78#issuecomment-16233127
.

@gazpachoking
Copy link
Contributor Author

The way I have it now puts both names onto the class, should it only add the canonical name? That might complicated things a bit.

BTW, do you prefer _checks_drafts like I have it, or the keyword idea?

@Julian
Copy link
Member

Julian commented Apr 11, 2013

Slight preference for the keyword way, but it's private so I don't really
care either way too strongly.

And yeah I saw both names are going to be on the class, but we should have
a table somewhere in the documentation where we mention format validation
and that table should list a single name for simplicity I think.
On Apr 11, 2013 9:16 AM, "Chase Sterling" notifications@github.com wrote:

The way I have it now puts both names onto the class, should it only add
the canonical name? That might complicated things a bit.

BTW, do you prefer _checks_drafts like I have it, or the keyword idea?


Reply to this email directly or view it on GitHubhttps://github.com//issues/78#issuecomment-16233571
.

@gazpachoking
Copy link
Contributor Author

Sounds good.

@Julian Julian closed this as completed Apr 14, 2013
Julian added a commit that referenced this issue Mar 22, 2015
0b657e8 Merge pull request #86 from kylef/patch-1
1bd4151 [README] JSONSchema.swift uses these tests too
8f86716 Revert "Add jon, JSON parser for the fishshell."
db9c629 Merge pull request #82 from bucaran/patch-1
875fa42 Add jon, JSON parser for the fishshell.
64c556c Merge pull request #81 from s-panferov/patch-1
43105d4 Add new Rust library to the list
aa4d927 Merge pull request #80 from seagreen/implementations
20ef067 Add new haskell implementation.
1274070 Merge pull request #79 from Muscula/json-schema-benchmark
6d8cf45 Merge pull request #78 from JamesNK/patch-1
55c4992 Add json-schema-benchmark to list of users of this test suite
645623d Added Newtonsoft.Json.Schema implementation
a7944d1 Merge pull request #76 from Prestaul/patch-1
5729cdf Added skeemas to list of suite users
4600fe3 Make the implementation list a bit less unwieldy now that it's so long (hooray!)
11d6905 Merge remote-tracking branch 'mafintosh/patch-1' into develop
689b80f Merge pull request #74 from bugventure/develop
c36f888 Add request-validator as a user of the test suite
41876b1 Update README.md
aabcb34 Merge pull request #71 from seagreen/additionalproperties
b3d160b Add tests for additionalProperties by itself.

git-subtree-dir: json
git-subtree-split: 0b657e8b0d21a4099d3ce378ba7208073a9295e6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Some new desired functionality
Projects
None yet
Development

No branches or pull requests

2 participants