Restricting configuration helpers to only working for a single command.

[rorticus]

Type: bug / feature

The following has been addressed in the PR:

• [x] There is a related issue
• [x] All code matches the style guide
• [x] Unit or Functional tests are included in the PR

Description:

Modifies the configuration helper to not allow it to be called with a command name. It is now always called with the group and command of the command being run.

I'm not really convinced on this, as it seems to me that it was a lot of code to change for one simple task, but I think I like the direction it's going.

Please give me your feedback!

Resolves #128

[codecov[bot]]

Codecov Report

Merging #137 into master will increase coverage by 0.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #137      +/-   ##
==========================================
+ Coverage   98.92%   98.94%   +0.01%     
==========================================
  Files          15       15              
  Lines         373      378       +5     
  Branches       44       45       +1     
==========================================
+ Hits          369      374       +5     
  Partials        4        4

Impacted Files	Coverage Δ
src/CommandHelper.ts	100% <100%> (ø)	:arrow_up:
src/Helper.ts	100% <100%> (ø)	:arrow_up:
src/configurationHelper.ts	100% <100%> (ø)	:arrow_up:
src/commands/eject.ts	96.07% <100%> (+0.07%)	:arrow_up:
src/loadCommands.ts	97.29% <100%> (ø)	:arrow_up:
src/registerCommands.ts	100% <100%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c797396...b078b74. Read the comment docs.

[agubler]

@tomdye can we get a review on this please?

Also @rorticus is this a breaking change?

[rorticus]

@agubler I'm afraid this is a breaking change. The helper coming into commands is modified to remove the command name parameters, so

configurationHelper.save("cli-eject", { someValues });

turns into,

configurationHelper.save({ someValues });

[agubler]

@rorticus Do you think we could maintain backwards compatibility by continuing to support both configurationHelper.save("cli-eject", { someValues }); & configurationHelper.save({ someValues });?

For the original API we could just change the implementation to ignore the first argument when there are two arguments passed? And mark it as deprecated?

[rorticus]

OK @agubler it looks like I was wrong. The original signature is,

configurationHelper.save({ someValues }, "cli-eject");

so it would be backwards compatible. I did however add the old signatures to the new methods and marked them as deprecated in the JSDOC.

[dylans]

This one needs a re-review and hopefully should be landed.