Thanks. Yah, I held off on tackling the update script since it wasn't used by the CLI tooling.
Now it will be easier to write test for the tooling as well :) On Wed, Sep 11, 2013 at 3:25 PM, Andrew Grieve <[email protected]> wrote: > Yep, I removed that when I checked in the directory. > > Nice work on the scripts Benn! Finally have now looked at them :). Planning > on converting the update script to follow the same pattern. > > > On Wed, Sep 11, 2013 at 3:50 PM, Benn Mapes <[email protected]> wrote: > > > Ok, but if we're checking in the node modules we should remove that bit > at > > the top of the create script that does an automatic 'npm install' if the > > modules aren't found, I had added that in so as not to break the cli. > > > > > > On Wed, Sep 11, 2013 at 12:27 PM, Brian LeRoux <[email protected]> wrote: > > > > > Ya, vendoring/shrink wrapping considered best practice in Node land. > > > On Sep 11, 2013 10:33 AM, "Andrew Grieve" <[email protected]> > wrote: > > > > > > > Yeah, I just did that yesterday. That was my main feedback to Benn > when > > > he > > > > announced the conversion to node for the android scripts. > > > > > > > > Rule of thumb is when you're distributing a tool, you should check in > > > your > > > > node_modules. Our build scripts fit into this bucket I think. > > > > > > > > > > > > On Wed, Sep 11, 2013 at 1:10 PM, Joe Bowser <[email protected]> > wrote: > > > > > > > > > It's in bin/node_modules. I understand the need for this for build > > > > > scripts, but shouldn't we tell people to npm update? > > > > > > > > > > On Wed, Sep 11, 2013 at 10:08 AM, Braden Shepherdson > > > > > <[email protected]> wrote: > > > > > > Where is this? That sounds odd. > > > > > > > > > > > > > > > > > > On Wed, Sep 11, 2013 at 1:06 PM, Joe Bowser <[email protected]> > > > wrote: > > > > > > > > > > > >> Any reason why? I'm wondering if this was done to help people > who > > > > > >> don't use node or something. > > > > > >> > > > > > > > > > > > > > > >
