Double-check API

[davidtheclark]

As of this morning I think I've built in all the features I thought were needed. I'm wondering if anybody has feedback on this API before I cut 1.0.

@jeddy3, @MoOx, @TrySound, @ai --- any thoughts? or anybody else you know that might be interested in using this so should check it out?

[davidtheclark]

cc/ @keithamus

[keithamus]

It looks to do what I want from it 😄.

However, I would suggest that it does not parse process.argv, and instead I have to manually pass you the option which I have parsed from the command line. I wouldn't want it to change behaviour through external options - only the options I feed into the function. But it's your code so you do what you want 😉

[TrySound]

@keithamus Maybe make it optional? @davidtheclark Is it optional?

[davidtheclark]

@keithamus : Thanks for the feedback. I was on the fence about --config and kind of did it just because rc was doing it so I assumed people liked it .... I do admit, though, that I can't think of a reason not to push the process.argv parsing into the user's court.

@TrySound : It's not optional currently but would be very easy to make optional.

[TrySound]

@davidtheclark I think every feature should be optional

[davidtheclark]

@TrySound : So you'd suggest lots more options --- for turning off YAML, JSON, or JS parsing, not looking in package.json, not looking for module.config.js, etc?

[TrySound]

@davidtheclark I'm about searching rc, config, global and argv. Parsing don't need to complicate.

[TrySound]

@davidtheclark Also I don't understand, why need parse-json if yaml can parse json out of the box. And first symbol checking do not make guarantee correct detecting.

[davidtheclark]

@TrySound : parse-json throws extra nice JSON errors, so I thought it might be handy. More importantly, though, I was finding that YAML was weirdly lenient in its JSON parsing? Maybe I misunderstood --- could test it out more. I agree that the first symbol checking is not great: open to a better way.

[TrySound]

@davidtheclark I think it's a good feature for rc file. It's multiformat file. So, why we shoundn't let it be free?

[TrySound]

@davidtheclark And need to add appveyor support. On windows tests fails

[MoOx]

1.0 looks good to me!

[TrySound]

@davidtheclark I'd like to suggest you to use this publishing way https://github.com/sindresorhus/np/blob/master/np.sh
Also if you use npm3 you can add some good hooks
https://github.com/TrySound/postcss-inline-svg/blob/master/package.json#L37-L42

[keithamus]

FYI those scripts also work for npm2. In fact they also work for npm1 😉. http://blog.keithcirkel.co.uk/how-to-use-npm-as-a-build-tool/

[TrySound]

@keithamus Oh, cool. I just heard from somebody that they are not reliable, but in npm3 works fine. Maybe just a noise in my head :))

[keithamus]

They're pretty darn reliable. I've been using them for about 2 years now, so long before npm3 came out. Never had a problem with them.

@davidtheclark you could also look at https://github.com/semantic-release/semantic-release - which takes a while to set up, but could manage all of your releases automatically for you 🎉

[davidtheclark]

I've never been too annoyed by typing the git and npm commands manually, but will look into the scripts. Thanks for the suggestions!

And thanks for all the feedback above. Based on what's been said so far, here are some changes I think could be made:

• Add noJs, noRc, noPackageProp, and noArgv options, each of which turns off one of the places this module might look.
• Add appveyor CI.

@TrySound I prefer keeping JSON strict instead of the leniency allowed by yaml parsing. If I have a .json file, I want it to be valid JSON. I will double-check my reasoning for including parse-json, though, because I don't exactly remember the details :)

[TrySound]

@davidtheclark Maybe better {js:true, rc:true, packageProp:true, argv:true, strictJSON:false} ?

[ben-eb]

I find positive options to be less confusing than negative ones.

[davidtheclark]

Good suggestion.

[TrySound]

@davidtheclark I didn't mean json files parsing with yaml. Only .[moduleName]rc file. It should have free format.

[TrySound]

@davidtheclark Can argv extends file config? Like here

[TrySound]

@davidtheclark However let's do not extend. Just rewrite. If somebody want to extend it he can disable default argv.

[davidtheclark]

That sounds like a good feature, yep.

[davidtheclark]

@TrySound : Actually, I'm second-guessing -- are you saying that if the configuration might have a format property, the module user could set or override that with --format something? I think that might be too app-specific: we don't want to check all the CLI arguments by default and plug them into the config. Considering how simple it should be for the cosmiconfig user to handle CLI arguments themselves and determine exactly what should get included in the config, vs what should do something else, I'm not sure cosmiconfig should try to address that.

[davidtheclark]

By closing #11 and #12 I think all the feedback above is addressed. If anybody feels like taking a second look at the docs to reassess, feedback would be much appreciated -- just open an issue!

[davidtheclark]

Also current version is published as 0.5.0, if you want to try it out and see if it does the job.