Skip to content

Add a arity predicate for functions#787

Merged
Marsup merged 1 commit intohapijs:masterfrom
AdrieanKhisbeArchives:check-function-arity
Dec 28, 2015
Merged

Add a arity predicate for functions#787
Marsup merged 1 commit intohapijs:masterfrom
AdrieanKhisbeArchives:check-function-arity

Conversation

@AdrieanKhisbe
Copy link
Contributor

add a predicate to enfore number args should take provided function.

This is fully functional, but predicate for min/max should be added to.

Waiting for some feedback for this implementation, (and suggestion for name. (so far thinking about maxArity, minArity

Copy link
Collaborator

Choose a reason for hiding this comment

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

const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted:
I copy pasted from number.min,
migt be then interesting to go over all the api.md to replace all the var.
(in a dedicated PR of course)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no const so far in the doc.
then should be changed all at once. :)

@Marsup
Copy link
Collaborator

Marsup commented Dec 27, 2015

Agreed on names.

@AdrieanKhisbe
Copy link
Contributor Author

voilà :)

test/function.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should check the error thrown in case it's not what you expect.

@AdrieanKhisbe
Copy link
Contributor Author

voilà :)

went for Error. But tell me if there is a Joi validation specific error type.

@Marsup
Copy link
Collaborator

Marsup commented Dec 27, 2015

That' fine, only missing arrow functions now.

@AdrieanKhisbe
Copy link
Contributor Author

dooooh, forgot about them. haha.
Will be done in few hours, have to go

@AdrieanKhisbe
Copy link
Contributor Author

done :)

test/function.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you check the reason of the failure for those as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for ['', false] you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment is on that line so yes :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha, git diff is too broad, showing the whole hunk ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(by the way, what does the null is a placeholder for?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

et voila.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Marsup
Copy link
Collaborator

Marsup commented Dec 28, 2015

Mind to squash those commits ?

Add a arity predicate for functions

Apply style recommandations for func().arity()

Add minArity and maxArity matchers to func()

function arity -> Made test more secific about exception thrown

Test also function arrity predicate on arrow functions

Add test predicated for error message in function
@AdrieanKhisbe
Copy link
Contributor Author

done

@Marsup Marsup changed the title WIP: Add a arity predicate for functions Add a arity predicate for functions Dec 28, 2015
@Marsup Marsup added the feature New functionality or improvement label Dec 28, 2015
@Marsup Marsup self-assigned this Dec 28, 2015
@Marsup Marsup added this to the 7.2.0 milestone Dec 28, 2015
Marsup added a commit that referenced this pull request Dec 28, 2015
@Marsup Marsup merged commit 004a3fd into hapijs:master Dec 28, 2015
@AdrieanKhisbe AdrieanKhisbe deleted the check-function-arity branch December 28, 2015 09:53
@AdrieanKhisbe AdrieanKhisbe restored the check-function-arity branch December 29, 2015 10:00
@AdrieanKhisbe AdrieanKhisbe deleted the check-function-arity branch December 29, 2015 10:01
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants