strongloop / strong-pm

deployer for node applications
http://strong-pm.io
Other
1k stars 71 forks source link

update ctl handler to use instance env #191

Closed rmg closed 9 years ago

rmg commented 9 years ago

Fixes compatibility with 3.x clients, like Arc 1.x

Connect to strongloop-internal/scrum-nodeops#512

rmg commented 9 years ago

There's a deeper concern here.. the code was obviously bad, and yet the pmctl-env tests were passing.. The meshctl client being tested no longer triggers the code in lib/ctl.js, so we have a test coverage gap.

sam-github commented 9 years ago

Does this fix https://github.com/strongloop-internal/scrum-nodeops/issues/512 ?

sam-github commented 9 years ago

We have many test coverage gaps. Some are described in https://github.com/strongloop-internal/scrum-nodeops/issues/495

sam-github commented 9 years ago

This change is easily unit-testable using a mock server passed to lib/ctl with an env-get request.

Merge it right away if its blocking people, but there is something quite odd here. In particular, I think there might be at least a comment needed on why the || 1, because a reader of the current code would assume its unreachable.