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

isAtLeast, isAtMost assertions added to assert interface #500

Merged
merged 1 commit into from
Jul 24, 2015

Conversation

wraithan
Copy link

I noticed the assert interface was missing these methods. One can use the operator method but that feels clunky. The CONTRIBUTING.md says to not expand the interface without asking due to possible rejection. I'm 100% fine with this being rejected if the expansion of the interface is deemed not useful. I attempted to match style best I could but there is no linter so I'm not 100% positive I've hit every point.

Also, it'd be nice if the CONTRIBUTING.md was expanded to discuss whether one should include an updated chai.js or not. I assumed not since that appeared to be part of the release process but I wasn't sure.

I went with is* for these to match the rest of the assert, I'm happy to rename them to something else that y'all like. I started with most and least like the should/expect interfaces but decided matching the interface I was adding them to was more important than matching what the other interfaces call them.

I also opted to not add the aliases gte and lte as there didn't appear to be any other shorthand aliases in assert. Happy to add those as well if those are desired.

Thanks for ChaiJS

@keithamus
Copy link
Member

Hey @wraithan thanks for the PR, good work 👍

keithamus added a commit that referenced this pull request Jul 24, 2015
isAtLeast, isAtMost assertions added to assert interface
@keithamus keithamus merged commit 7d36b5a into chaijs:master Jul 24, 2015
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