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(validator): Forbidden rule does not support float #449

Closed
wants to merge 1 commit into from
Closed

fix(validator): Forbidden rule does not support float #449

wants to merge 1 commit into from

Conversation

@safaozturk93
Copy link

@safaozturk93 safaozturk93 commented Sep 30, 2018

#436

Copy link
Member

@funkyfuture funkyfuture left a comment

there must be at least tests covering the currently undesired behaviour (w/ the intended outcome).

@@ -70,7 +70,7 @@
# TODO remove KEYSCHEMA AND VALUESCHEMA with next major release
KEYSRULES = KEYSCHEMA = ErrorDefinition(0x83, 'keysrules')
VALUESRULES = VALUESCHEMA = ErrorDefinition(0x84, 'valuesrules')
BAD_ITEMS = ErrorDefinition(0x8f, 'items')
BAD_ITEMS = ErrorDefinition(0x8F, 'items')
Copy link
Member

@funkyfuture funkyfuture Sep 30, 2018

that is an unnecessary change.

Copy link
Author

@safaozturk93 safaozturk93 Sep 30, 2018

this violates black reformatting rule in travis pipeline.

@@ -1189,6 +1189,11 @@ def _validate_forbidden(self, forbidden_values, field, value):
elif isinstance(value, int):
if value in forbidden_values:
self._error(field, errors.FORBIDDEN_VALUE, value)
elif isinstance(value, float):
Copy link
Member

@funkyfuture funkyfuture Sep 30, 2018

please keep it simple. afaict, there's no need to branch off for float or integer values, only for container values that are not strings.

Copy link
Author

@safaozturk93 safaozturk93 Sep 30, 2018

I will look at it.

@funkyfuture funkyfuture added this to the 1.2.1 milestone Sep 30, 2018
funkyfuture added a commit to funkyfuture/cerberus that referenced this issue Nov 24, 2018
funkyfuture added a commit to funkyfuture/cerberus that referenced this issue Nov 24, 2018
@funkyfuture
Copy link
Member

@funkyfuture funkyfuture commented Jan 27, 2019

Closing in favour of #455.

funkyfuture added a commit that referenced this issue Jan 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants