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 for https://github.com/networknt/light-rest-4j/issues/64 #106

Merged
merged 5 commits into from
Dec 12, 2018

Conversation

BalloonWen
Copy link
Contributor

-this PR is part of the fix for networknt/light-rest-4j#64
-BaseValidator will have a new property "ValidatorConfig", this ValidatorConfig will be set when constructing from the constructor, get from ValidationContext. Its child validator "TypeValidator" will use this property to get the config.
-"VaidatorConfig" will be set into "ValidationContext" when creating new JsonSchema using "newJsonSchema()" in "JsonSchemaFactory"

@BalloonWen BalloonWen added the pending review don't merge for now label Dec 11, 2018
@BalloonWen
Copy link
Contributor Author

renamed ValidatorConfig to SchemaValidatorsConfig based on comments of networknt/light-rest-4j#65

@stevehu
Copy link
Contributor

stevehu commented Dec 12, 2018

Any reason to use plural instead of singular? SchemaValidatorsConfig vs SchemaValidatorConfig.

@BalloonWen
Copy link
Contributor Author

Any reason to use plural instead of singular? SchemaValidatorsConfig vs SchemaValidatorConfig.

Because this config obj is in BaseJsonValidator. Any validator extends this BaseJsonValidator will have this config, so I think this config obj can contain configs not only for TypeValidator but can also have configs for other validators. What do you think?

@stevehu
Copy link
Contributor

stevehu commented Dec 12, 2018

Good. It is just a name and I am just curious. Thanks.

@stevehu stevehu merged commit a2b194c into networknt:develop Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending review don't merge for now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants