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(multi): item rule has custom checker will throw error if validate… #290

Merged
merged 4 commits into from
Mar 21, 2022

Conversation

0x0a0d
Copy link
Contributor

@0x0a0d 0x0a0d commented Mar 15, 2022

… failed

custom checker push errors to fnCustomErrorsX that will be assigned to var errors, not _errors as expected
To avoid a breaking changes, I just keep size of errors before validate and restore it if failed
But I think the logic should be changed
image

… failed

custom checker push errors to fnCustomErrorsX that will be assigned to var errors, not _errors as expected
To avoid a breaking changes, I just keep size of errors before validate and restore it if failed
But I think the logic should be changed
@0x0a0d
Copy link
Contributor Author

0x0a0d commented Mar 15, 2022

I add some test that will break with current master branch

test/rules/multi.spec.js Outdated Show resolved Hide resolved
test/rules/multi.spec.js Outdated Show resolved Hide resolved
@icebob icebob requested a review from erfanium March 16, 2022 18:41
@0x0a0d 0x0a0d requested a review from icebob March 16, 2022 23:00
@0x0a0d
Copy link
Contributor Author

0x0a0d commented Mar 18, 2022

@erfanium pls fix this too

Copy link
Owner

@icebob icebob left a comment

Choose a reason for hiding this comment

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

Thanks!

@icebob icebob merged commit a746f93 into icebob:master Mar 21, 2022
@0x0a0d 0x0a0d deleted the multi_rule_with_custom_checker branch March 22, 2022 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants