Default value of the config option

[markelog]

By default, config option uses .jscs.json value, which might be implicitly merged with inline options.

It might be better to change its default value to null, like jshintrc option for grunt-contrib-jshint, so user could define this option explicitly then otherwise.

But that would be a breaking change :-(

[gustavohenke]

What about inserting this change in a 0.5.x version?
We could add a warning about it in 0.4.0, then in 0.5.0 the default value is changed to empty.

However I'm not sure if this is semantic versioning 😕

[markelog]

Agreed.

[mikesherov]

Looks like this was addressed in: 0f1ac1a

[gustavohenke]

Not really, we plan to change it to null in v0.5.0. I've left this issue open as a reminder when v0.5.0 gets released.

[mikesherov]

Great, thanks.

[mikesherov]

@gustavohenke is this going to get into this version you're about to release?

[gustavohenke]

I'm now seeing the mention in jscs-dev/node-jscs#379 by @markelog, unfortunately I had ignored that issue :(

Well, thinking more about it, jscs assumes that no config defaults to .jscsrc, what we've being doing since the begining. What do you guys think about it?

[mikesherov]

I'm fine either way. It's up to @markelog

[gustavohenke]

I've been thinking of adding a explicit warning instead of using null as default value for v0.5.0, and then we postpone tha change again to v0.6.0.
@mikesherov @markelog opinions?

[gustavohenke]

Well, I ended up opting for the original proposal.

[gustavohenke]

BTW, v0.5.1 is out (0.5.0 lacked the readme changes... d'oh!)

[Krinkle]

I'm confused. I just tried to upgrade from 0.4.4 to 0.5.1 and it seems it no longer works as expected. I'm aware of this thread but it's sufficiently vague that I'm not certain whether this change was intended to work out this way.

I used to hardcode the path to .jshintrc and jscs.json. When jscs changes its standard to .jscsrc I decided to follow suit and in addition no longer hardcode it. The auto-discovery works great, and matches the behaviour of standalone node-jscs, and is still quite stable because the package versions are fixed in package.json (if the default changes again, I can update it if/when I'm ready).

Same for grunt-contrib-jshint. I used to specify the path to it (.jshintrc) but didn't like having to override it for subdirectories and specifying true there now allows the jshint package to use its discovery instead. And again, it matches behaviour of the standalone jshint cli.

However it seems grunt-jscs-checker is quite the opposite as I'm now required to explicitly specify the file?

Running the jscs task (as of v0.5.1) with only a list of files to check:

- src/
- test/
- .jscsrc
- Gruntfile.js
- package.json

        jscs: {
            dev: [
                '*.js',
                '{src,test}/**/*.js'
            ]
        },

...this passed with v0.4.4, but fails by default under v0.5.1:

$ grunt jscs --verbose
(..)
Running tasks: jscs

Running "jscs" task

Running "jscs:dev" (jscs) task
Verifying property jscs.dev exists in config...OK
Files: Gruntfile.js, src/EventEmitter.js, src/core.js, src/util/jquery.js, test/unit/EventEmitter.test.js, test/unit/core.test.js, test/unit/util.test.js -> dev
Options: (none)
Fatal error: Nor config file nor inline options weren't found

[gustavohenke]

I think we can accept true to use default behaviour from jscs.
Would this work for you?

[mikesherov]

I'm confused too. I was under the impression that removing .jscsrc just meant we'd fall back to JSCS's default, which is .jscsrc.

[gustavohenke]

Or maybe I got the message wrong.
By default, grunt-contrib-jshint doesn't use any configurations if jshintrc option/config options aren't defined.

So, the purpose of not having a default value would be to not use .jscsrc unless it's defined in the jscs target.

@mikesherov, @Krinkle and @markelog, what are your opinions about using true, just like grunt-contrib-jshint?

[Krinkle]

I'd say that's one way to expose the default logic (it seems right now there is no way to trigger it, no matter how, I think it should be exposed in some way). It depends on what we think makes for a better default.

I'd say it's more common and a best practice to have the options in a file (so that e.g. text editor plugins and other scripts can simply invoke node-jscs and let it be used). And not requiring to specify true would also make it compatible previous versions of grunt-jscs-checker.

We might not even need to have a way to disable it completely. Though if there's use for it, we could use null as a way to disable loading of any config file and use true (which would be the default) to have node-jscs find one.

[gustavohenke]

Well. Thinking better about it now, it makes more sense to have a default.

grunt-contrib-jshint doesn't have a default because jshint uses some configurations by default (like expr: false, but jscs does no style checkings if a configuration is not defined.

Let's do this way then:

• Falsy values (but not undefined) will completely disable loading of any config file (not only null);
• true or undefined will use the default .jscsrc;
• Any string value point to the config file name.

[mikesherov]

I think the last proposal here makes most sense.

Sorry for any confusion!

[gustavohenke]

I'll work on this one soon.
Let me know if anyone wants to work on it.

[gustavohenke]

Working on this issue.