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

Rename be_true and be_false to be_truthy and be_falsey. #284

Merged

Conversation

@penelopezone
Copy link
Member

@penelopezone penelopezone commented Jul 7, 2013

See: #283.

Signed-off-by: Sam Phippen <samphippen@googlemail.com>
@cupakromer
Copy link
Member

@cupakromer cupakromer commented Jul 7, 2013

Do we want to deprecate be_true and be_false first while adding be_truthy and be_falsey as aliases?

@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 7, 2013

Yes. I'll do that in 2-99 if this gets approval 💣

@alindeman
Copy link
Contributor

@alindeman alindeman commented Jul 7, 2013

I thought be_true and be_false were staying, but now mean == true and == false.

@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 7, 2013

@alindeman #283 calls for deprecation then removal, @myronmarston also pointed out that one can do be true and be false to get that behaviour.

@alindeman
Copy link
Contributor

@alindeman alindeman commented Jul 7, 2013

Perfect, that makes sense.

@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 10, 2013

@alindeman can you take a look at the 2-99 pull and then tell me if you think this is good for mergification?

@alindeman
Copy link
Contributor

@alindeman alindeman commented Jul 14, 2013

We discussed an alias for be_falsy. I don't see that in this pull yet.

Sam Phippen added 2 commits Jul 21, 2013
Sam Phippen
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 21, 2013

@alindeman and fixed.

@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 21, 2013

Also about to update the 2.99 pull.

@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 21, 2013

@JonRowe @soulcutter @alindeman @myronmarston if one you see a reason not to merge this and the equivalent pull for 2-99 (#285) before I get up tomorrow, I'll fix it. Otherwise I'll merge tomorrow morning.

@coveralls
Copy link

@coveralls coveralls commented Jul 21, 2013

Coverage Status

Coverage increased (+0%) when pulling 4e4ef45 on samphippen:change-be-true-false-to-truthy-falsey into feeb27d on rspec:master.

penelopezone pushed a commit that referenced this pull request Jul 22, 2013
…y-falsey

Rename be_true and be_false to be_truthy and be_falsey.
@penelopezone penelopezone merged commit 42d0bb8 into rspec:master Jul 22, 2013
1 check passed
1 check passed
default The Travis CI build passed
Details
@JonRowe
Copy link
Member

@JonRowe JonRowe commented Jul 22, 2013

Hey so obviously this broke the rest of rspec's test suites, but there's also a side effect be_true is still a valid rspec statement (it now goes to the be_* matcher to assert on true?) are we ok with this?

@penelopezone
Copy link
Member Author

@penelopezone penelopezone commented Jul 22, 2013

@JonRowe true? and false? can be defined by users, but they aren't defined on Object so I don't think this is a problem.

@JonRowe
Copy link
Member

@JonRowe JonRowe commented Jul 22, 2013

@samphippen my concern was more that given be_true and be_false have previously had specific meaning that it might be confusing for users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.