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

fixed expect('').to.contain('') so it passes #512

Merged
merged 1 commit into from Sep 11, 2015

Conversation

Projects
None yet
2 participants
@dereke
Contributor

dereke commented Sep 1, 2015

Found a bug when you are trying to assert against a blank string.
Test case and fix included

@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Sep 1, 2015

Member

Hey @dereke, thanks for the PR. Could you please rebase your commits so as not to include chai.js. It is built per-release and so shouldn't be built for individual PRs.

Member

keithamus commented Sep 1, 2015

Hey @dereke, thanks for the PR. Could you please rebase your commits so as not to include chai.js. It is built per-release and so shouldn't be built for individual PRs.

@dereke

This comment has been minimized.

Show comment
Hide comment
@dereke

dereke Sep 1, 2015

Contributor

Hi @keithamus when we ran the tests without having build chai.js they failed.
Does CI build chai.js when the tests run?

Contributor

dereke commented Sep 1, 2015

Hi @keithamus when we ran the tests without having build chai.js they failed.
Does CI build chai.js when the tests run?

@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Sep 1, 2015

Member

The CI does indeed run the tests: https://travis-ci.org/chaijs/chai/builds/78216305. Also, you're welcome to build chai.js as many times as you want locally, just don't commit it 😉

Member

keithamus commented Sep 1, 2015

The CI does indeed run the tests: https://travis-ci.org/chaijs/chai/builds/78216305. Also, you're welcome to build chai.js as many times as you want locally, just don't commit it 😉

@dereke

This comment has been minimized.

Show comment
Hide comment
@dereke

dereke Sep 1, 2015

Contributor

OK cool. all done.

Contributor

dereke commented Sep 1, 2015

OK cool. all done.

@dereke

This comment has been minimized.

Show comment
Hide comment
@dereke

dereke Sep 11, 2015

Contributor

@keithamus when do you think this will show up in a release?

Contributor

dereke commented Sep 11, 2015

@keithamus when do you think this will show up in a release?

@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Sep 11, 2015

Member

Thanks for reminding me @dereke. I'm going to look at this in a couple of hours and make a release in about 5 hours, which will be 3.2.0 (hopefully)

Member

keithamus commented Sep 11, 2015

Thanks for reminding me @dereke. I'm going to look at this in a couple of hours and make a release in about 5 hours, which will be 3.2.0 (hopefully)

keithamus added a commit that referenced this pull request Sep 11, 2015

Merge pull request #512 from dereke/master
fixed expect('').to.contain('') so it passes

@keithamus keithamus merged commit cf9f5d9 into chaijs:master Sep 11, 2015

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@dereke

dereke Sep 15, 2015

Contributor

any chance of publishing the new package?

Contributor

dereke commented Sep 15, 2015

any chance of publishing the new package?

@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Sep 21, 2015

Member

@dereke this should be in 3.3.0

Member

keithamus commented Sep 21, 2015

@dereke this should be in 3.3.0

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