dojo / cli

:rocket: Dojo - command line tooling.
http://dojo.io
Other
25 stars 34 forks source link

Fix: correctly load .dojorc commands #149

Closed mwistrand closed 7 years ago

mwistrand commented 7 years ago

Type: bug

The following has been addressed in the PR:

Description:

Updates the cli helper to take a simplified command name (e.g., "webpack" or "intern") instead of a composite of the group and command (e.g., "build-webpack" or "test-intern"), so that when looking up data from a .dojorc, the correct key is used (e.g., "build-webpack" instead of just "build" or "build-build-webpack").

Verified with @dojo/examples/todo-mvc-kitchensink, using both the $ dojo build and $ dojo test cli commands. Note that @dojo/cli-test-intern currently does not ask for .dojorc data, but a quick change to the local source confirmed that these changes will work whenever that update is made.

Resolves #148

codecov[bot] commented 7 years ago

Codecov Report

Merging #149 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
+ Coverage   98.94%   98.94%   +<.01%     
==========================================
  Files          15       15              
  Lines         378      379       +1     
  Branches       45       46       +1     
==========================================
+ Hits          374      375       +1     
  Partials        4        4
Impacted Files Coverage Δ
src/loadCommands.ts 97.29% <100%> (ø) :arrow_up:
src/registerCommands.ts 100% <100%> (ø) :arrow_up:
src/CommandHelper.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 eac377f...feb3fdf. Read the comment docs.

agubler commented 7 years ago

@mwistrand surprised it didn't break any tests but perhaps it was because the test data was actually wrong.

mwistrand commented 7 years ago

@agubler Resolving #150 might uncover just that.