Closed kraman closed 10 years ago
Changes since 2e9d16f:
No need to change doGitPush since callback is being called by shelljs.exec
@sam-github @rmg Please review
Description line should be:Deploy a node application to a StrongLoop process manager
On Sat, Jul 19, 2014 at 6:23 PM, Krishna Raman notifications@github.com wrote:
@sam-github https://github.com/sam-github @rmg https://github.com/rmg Please review
— Reply to this email directly or view it on GitHub https://github.com/strongloop/strong-deploy/pull/1#issuecomment-49534325 .
Chanda Dharap Director, Engineering @StrongLoop Inc (510) 304-3924 chanda@strongloop.com
StrongLoop http://strongloop.com/* makes it easy to develop APIs http://strongloop.com/mobile-application-development/loopback/ in Node, plus get DevOps capabilities http://strongloop.com/node-js-performance/strongops/ like monitoring, debugging and clustering*.
LGTM
There's some amount of judgement here, but FWIW, if you had pushed either a series of commits for the things in your bullet list with the bullets as subjects (preceeded with "fixup!"), or just one commit that did it all, then I'd just be able to review the diff from the previous version I reviewed. Then you could rebase before merge and squash the fixes in.
With this way, I have to re-review the entire file top-to-bottom from scratch.
github semi-supports this, since code that gets changed in a new small fixup commit will cause any comments on that code in a previous version to dissappear, so its a light weight way to track that every comment has gotten addressed.
I say some amount of judgement, because sometimes there are changes on master, for example, that you really want in your branch, and so have to rebase onto head of master to get them, and then all the comments get lost, but that's better than fixing things that are already fixed on master. Probably other examples, too.
@sam-github Sure. Will make fixup commits next time. In this case part of what I did was split up one commit into 2 so your comments were lost.
Also see: https://github.com/strongloop/strong-cli/pull/149