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

Various work on aliases #489

Merged
merged 7 commits into from Jul 19, 2015

Conversation

Projects
None yet
2 participants
@astorije
Member

astorije commented Jul 17, 2015

Here is what this PR does:

  • Add is* aliases for extensible, notExtensible, sealed, notSealed, frozen, notFrozen and update their respective tests to test the aliases
  • Add is* aliases for ok and notOk and update their respective tests to test the aliases
  • Make the use of throws consistent: before these changes, the name of the assertion in the documentation was throws, the examples were using throw and the code was using Throw. Now it's throws everywhere and the rest is aliases
  • Add tests for aliases of throws (throw and Throw)
  • Since we already had isTrue, isNull, isObject, isBoolean, ... (20 in total), I made the is* aliases the authoritative source, and kept the original ones as aliases for backward-compatibility. For example, isOk should be the "official" version (like isTrue), but ok still exists. If I am not committing any typo, nothing in the API will be broken, just the way it's presented is more consistent.

This PR doesn't update the main chai.js file.

@astorije

This comment has been minimized.

Show comment
Hide comment
@astorije

astorije Jul 17, 2015

Member

I can also add tests for theses aliases, to make sure they run OK. Let me know if you want that.

Member

astorije commented Jul 17, 2015

I can also add tests for theses aliases, to make sure they run OK. Let me know if you want that.

@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Jul 18, 2015

Member

If you would kindly add tests, then I'll happily merge 😄

Member

keithamus commented Jul 18, 2015

If you would kindly add tests, then I'll happily merge 😄

@astorije astorije changed the title from Aliases to frozen/notFrozen, sealed/notSealed and extensible/notExtensible to Various work on aliases Jul 18, 2015

@astorije

This comment has been minimized.

Show comment
Hide comment
@astorije

astorije Jul 18, 2015

Member

@keithamus here you go with the tests... I also took some liberties as you can see, making this PR broader but still focused on aliases for the assert API. I believe it makes things better by adding consistency and tests, I hope you're gonna like it :)

Let me know if you need anything else on this!

Member

astorije commented Jul 18, 2015

@keithamus here you go with the tests... I also took some liberties as you can see, making this PR broader but still focused on aliases for the assert API. I believe it makes things better by adding consistency and tests, I hope you're gonna like it :)

Let me know if you need anything else on this!

@astorije

This comment has been minimized.

Show comment
Hide comment
@astorije

astorije Jul 18, 2015

Member

Note that since there are a lot of changes, I made sure individual commits were self-contained so that you can go through each of them instead of having to digest the resulting blob in one go!

Member

astorije commented Jul 18, 2015

Note that since there are a lot of changes, I made sure individual commits were self-contained so that you can go through each of them instead of having to digest the resulting blob in one go!

@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Jul 19, 2015

Member

LGTM 👍

Member

keithamus commented Jul 19, 2015

LGTM 👍

keithamus added a commit that referenced this pull request Jul 19, 2015

@keithamus keithamus merged commit 40cdaa3 into chaijs:master Jul 19, 2015

1 check passed

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

@astorije astorije deleted the astorije:astorije/aliases branch Jul 19, 2015

@astorije

This comment has been minimized.

Show comment
Hide comment
@astorije

astorije Jul 19, 2015

Member

Thanks @keithamus!!

Member

astorije commented Jul 19, 2015

Thanks @keithamus!!

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