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

Added approximately alias to close to #527

Merged
merged 1 commit into from Oct 4, 2015

Conversation

Projects
None yet
2 participants
@christophertrml
Contributor

christophertrml commented Oct 4, 2015

Solves #525 , barring some questions.

  1. Should approximately be added to the assert interface as well? This PR does, but this can be removed if you feel it bloats the interface.
  2. The aliasing syntax doesn't allow for custom error messaging. This is fine for things like satisfy vs satisfies, but it gets a bit weird when it's something like closeTo and approximately. I edited the internal error messages to be more explicit ('the arguments to closeTo or approximately must be numbers'), but left the assertion messaging the same as to not break anything that might rely on that specific messaging. I don't really like this, because the closeTo function now knows how it's being used... but not having it can be really confusing for people trying to use approximately in a wrong way. What do you think of this? We could also just leave all messaging the same and treat approximately as a true alias.
@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Oct 4, 2015

Member
  1. assert should be a first class citizen - ideally it would have every method the other two interfaces have, including aliases. So we can keep this, good job!
  2. The precedent has been set with other assertion methods - we provide aliases but the messaging always leans on the original method name, but this could be documented better on our website, which is in much need of love anyway. Having said that, I think we can keep the early error regarding type info.

All is good, thanks for the hard work 😄

Member

keithamus commented Oct 4, 2015

  1. assert should be a first class citizen - ideally it would have every method the other two interfaces have, including aliases. So we can keep this, good job!
  2. The precedent has been set with other assertion methods - we provide aliases but the messaging always leans on the original method name, but this could be documented better on our website, which is in much need of love anyway. Having said that, I think we can keep the early error regarding type info.

All is good, thanks for the hard work 😄

keithamus added a commit that referenced this pull request Oct 4, 2015

Merge pull request #527 from cjqed/should-approximately
Added approximately alias to close to

@keithamus keithamus merged commit 6d0ae35 into chaijs:master Oct 4, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@renovate renovate bot referenced this pull request Feb 2, 2018

Open

Update dependency chai to v4 #30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment