Move values to options

[KATT]

Closes #7

[coveralls]

Coverage increased (+0.7%) to 100.0% when pulling 6875b6e on KATT:feature/move-values-to-options into b638df2 on Floby:master.

[Floby]

Hey ! I understand the reflexion. I'm thinking I'm gonna use this but only after publishing your other PRs with the correct semver bumps. This one is obviously a breaking change.

[Floby]

I'm not sure this does what needs to be done. Doesn't this assign options.values to process.env ?

[KATT]

It's a pretty normal way to deal with default values nowadays since destructuring is becoming the norm. This creates the variable values from options.values with the fallback of process.env.

From MDN web docs:

var {a = 10, b = 5} = {a: 3};

console.log(a); // 3
console.log(b); // 5

You can see that the tests pass too, so it definitely works.

[Floby]

I see, I mistook it for values: process.env

[Floby]

Same here, I'm not sure what this does. Even if it works I think i'd prefer a less cool but clearer approach

[KATT]

I understand it's confusing if it's the first time you read it - but it's clearer than

const values = typeof options.values === 'undefined' ? options.values : process.env;
const noDefaults = typeof options.noDefaults === 'undefined' ? false : options.noDefaults;

.. or to reassign options arg that you can see in a lot of old jQuery-plugins, etc

options = Object.assign({}, { 
  noDefaults: false, 
  values: process.env,
}, options || {});

[Floby]

I usually do

const values = options.values || process.env

which I find rather short and self-explanatory. But I mostly find it more used to!

[KATT]

This:

const values = options.values || process.env
const noDefaults = options.noDefaults || true

Won't work for booleans.. e.g. if we'd implement #8 it'd always be true, that's why the typeof is needed.

[KATT]

If you know how deconstructuring works (been around since node 6) the pattern I do in this PR is self-explanatory as well. Get with the times! :D

[KATT]

It's a breaking change indeed, so it'd need a major bump.

Also, can you reflect on #8 and leave a comment what you think? Also breaking, could go in the same major if you agree.