-
Notifications
You must be signed in to change notification settings - Fork 213
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
Deprecate isTrue, isFalse #2358
Comments
I'm of two minds about this. I see where you're coming from, but I also appreciate the aesthetic value of letting matchers read like sentences where possible—"expect foo is true" reads better than "expect foo true". If we do decide to do this, I'd want to first improve the output of |
Added to 1.0.0 milestone so we can either decide not to deprecate, or have them removed before the version bump. This has come up on mailing lists as a source of confusion, IMO having 1 way to solve this problem outweighs the aesthetics of the call. It definitely can't hurt to have nicer output from |
One advantage of expect/*Inferred: <bool>*/(someFlag, isTrue); That means if you accidentally type: expect('clearlyNotTrue', isTrue); We could potentially lint this. Just an idea. |
Hmm, yeah with #2352 these could potentially have more utility. Though if we work out the Do we see a world where we start requiring |
I personally use |
I think that's more about implicit vs. explicit use of
FWIW, I much prefer the existing output from |
Any plans on this? It always bothered me that we have two ways of accomplishing the same thing, and I myself often use both ways, in an inconsistent manner. It would be good to, at least, have a lint to improve consistency. |
No current plans. I think this would still need some careful consideration of the costs and benefits. I think there are folks who use these and might feel upset if we deprecate them. One workaround in the meantime might be to add a CI step that checks for the regex |
These matchers, when used in
expect
calls, don't add value over literal boolean values.If there aren't other places which is seeing value from these we should deprecate and eventually remove them so we don't have multiple ways of accomplishing the same thing.
The text was updated successfully, but these errors were encountered: