Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Check for config via NOT #39

Merged
merged 1 commit into from

2 participants

@maxrabin

Until yesterday, this was if (config == null), which checks for a certain
falseyness of config. This was changed yesterday to if (config === null) which
checks if config is, in fact, null. The problem is that when calling a function
without a parameter, then the paramter is undefined. So maybe we should check
for if (config === undefined) but I figure its easier and clearer to check
if (!config)

@maxrabin maxrabin Check for config via NOT
Until yesterday, this was `if (config == null)`, which checks for a certain
falseyness of `config`. This was changed yesterday to `if (config === null)` which
checks if `config` is, in fact, `null`. The problem is that when calling a function
without a parameter, then the paramter is `undefined`. So maybe we should check
for `if (config === undefined)` but I figure its easier and clearer to check
`if (!config)`
16a40ec
@maxrabin

0ba42d1#L3L19 is where this broke.

@alexvollmer
Owner
@alexvollmer
Owner

@maxrabin, did you verify this against a real project? I don't have one handy to check with for the moment. But if you verified the test() function both with and without additional config options I'll go ahead an merge this. Thanks again for the patch.

@maxrabin

Yes, I tested it both with and without config options, and with different values for the config, so I believe it is safe to merge in to master. Regarding a test app, I agree completely but I don't have such an app nor am I in a position to write one, (mainly because I don't know how. I got assigned to QA because I'm a Javascript developer).

@alexvollmer alexvollmer merged commit 7d3e238 into alexvollmer:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 30, 2012
  1. @maxrabin

    Check for config via NOT

    maxrabin authored
    Until yesterday, this was `if (config == null)`, which checks for a certain
    falseyness of `config`. This was changed yesterday to `if (config === null)` which
    checks if `config` is, in fact, `null`. The problem is that when calling a function
    without a parameter, then the paramter is `undefined`. So maybe we should check
    for `if (config === undefined)` but I figure its easier and clearer to check
    `if (!config)`
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  test.js
View
2  test.js
@@ -16,7 +16,7 @@
*
*/
function test(title, f, options) {
- if (options === null) {
+ if (!options) {
options = {
logTree: true
};
Something went wrong with that request. Please try again.