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

Primitive.notOneOf() Implementation #13

Merged
merged 4 commits into from
Jul 2, 2019

Conversation

jnaputi253
Copy link
Contributor

Here's the implementation for the method along with the tests for it. Let me know if any fixes are needed!

@jnaputi253 jnaputi253 changed the title #notOneOf() Implementation Primitive.notOneOf() Implementation Jul 1, 2019
@@ -61,6 +61,26 @@ abstract class NopePrimitive<T> implements IValidatable<T> {
return this.test(rule);
}

public notOneOf(options: Array<T | NopeReference | Nil>, message = 'Invalid Option') {
const resolveNopeRef = (option: T | NopeReference | Nil, context?: { [key: string]: any}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be possible to extract this to the ./utils.ts file for both oneOf and notOneOf pretty please? 😄

@bvego
Copy link
Collaborator

bvego commented Jul 2, 2019

The implementation is great, thanks! I left a small comment that I'd like to be resolved so we don't have duplicate code

Thanks! 😄

@jnaputi253
Copy link
Contributor Author

Went ahead and moved the duplicate code to the utils.ts file 😄

@bvego
Copy link
Collaborator

bvego commented Jul 2, 2019

Awesome, great job man! Thank you so much!

If you have any feature ideas or things that you would like to see feel free to open up an issue 😄

Merging!

@bvego bvego merged commit f0aee46 into ftonato:master Jul 2, 2019
@jnaputi253 jnaputi253 deleted the feature/notOneOf branch July 2, 2019 13:02
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.

None yet

2 participants