Re: Android platform scripts
(Finally back from two days of intern interviews and one day of writing feedback.) I would prefer to do it part by part, but this is difficult. If the callers of the method I port are expecting it to be synchronous, they'll continue on to the next step too early. If I only change the outermost callers, and not the callees, there's little benefit to this. Especially since these have no tests, I'd love to keep it small, but I can't see a way to do that without porting all of one command, at least. (And there are some interdependencies.) Braden On Fri, Sep 27, 2013 at 5:40 PM, Brian LeRoux wrote: > Giver. Now, instead if the grand rewrite how about a refactor of a single > method for review. Easier for us to buy in and perhaps collab on w you. > On Sep 27, 2013 7:31 PM, "Braden Shepherdson" wrote: > > > I had since learned that shelljs is used for other things. That's fine, > > it's not hurting anything unless we use synchronous exec. > > > > Braden > > > > > > On Fri, Sep 27, 2013 at 12:47 PM, Andrew Grieve > >wrote: > > > > > SGTM. shelljs is used for other things though, so we won't be able to > get > > > rid of it. > > > > > > > > > On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson < > bra...@chromium.org > > > >wrote: > > > > > > > The Android platform scripts use shelljs.exec's synchronous mode. > This > > > is a > > > > terrible hack that leaks filehandles by the hundred, wastes lots of > CPU > > > > cycles, and can cause EMFILE on OSX because it runs out of > filehandles. > > > > > > > > I wanted to rewrite the scripts to be async and use > child_process.exec > > or > > > > .spawn. I started to, but rapidly found that the tangle of callbacks > > that > > > > resulted was terrible and confusing. > > > > > > > > I propose using Q.js here as well (and dropping shelljs as a > > dependency, > > > if > > > > it was only ever used for .exec). > > > > > > > > Any objections? > > > > > > > > Braden > > > > > > > > > >
Re: Android platform scripts
Giver. Now, instead if the grand rewrite how about a refactor of a single method for review. Easier for us to buy in and perhaps collab on w you. On Sep 27, 2013 7:31 PM, "Braden Shepherdson" wrote: > I had since learned that shelljs is used for other things. That's fine, > it's not hurting anything unless we use synchronous exec. > > Braden > > > On Fri, Sep 27, 2013 at 12:47 PM, Andrew Grieve >wrote: > > > SGTM. shelljs is used for other things though, so we won't be able to get > > rid of it. > > > > > > On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson > >wrote: > > > > > The Android platform scripts use shelljs.exec's synchronous mode. This > > is a > > > terrible hack that leaks filehandles by the hundred, wastes lots of CPU > > > cycles, and can cause EMFILE on OSX because it runs out of filehandles. > > > > > > I wanted to rewrite the scripts to be async and use child_process.exec > or > > > .spawn. I started to, but rapidly found that the tangle of callbacks > that > > > resulted was terrible and confusing. > > > > > > I propose using Q.js here as well (and dropping shelljs as a > dependency, > > if > > > it was only ever used for .exec). > > > > > > Any objections? > > > > > > Braden > > > > > >
Re: Android platform scripts
I had since learned that shelljs is used for other things. That's fine, it's not hurting anything unless we use synchronous exec. Braden On Fri, Sep 27, 2013 at 12:47 PM, Andrew Grieve wrote: > SGTM. shelljs is used for other things though, so we won't be able to get > rid of it. > > > On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson >wrote: > > > The Android platform scripts use shelljs.exec's synchronous mode. This > is a > > terrible hack that leaks filehandles by the hundred, wastes lots of CPU > > cycles, and can cause EMFILE on OSX because it runs out of filehandles. > > > > I wanted to rewrite the scripts to be async and use child_process.exec or > > .spawn. I started to, but rapidly found that the tangle of callbacks that > > resulted was terrible and confusing. > > > > I propose using Q.js here as well (and dropping shelljs as a dependency, > if > > it was only ever used for .exec). > > > > Any objections? > > > > Braden > > >
Re: Android platform scripts
SGTM. shelljs is used for other things though, so we won't be able to get rid of it. On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson wrote: > The Android platform scripts use shelljs.exec's synchronous mode. This is a > terrible hack that leaks filehandles by the hundred, wastes lots of CPU > cycles, and can cause EMFILE on OSX because it runs out of filehandles. > > I wanted to rewrite the scripts to be async and use child_process.exec or > .spawn. I started to, but rapidly found that the tangle of callbacks that > resulted was terrible and confusing. > > I propose using Q.js here as well (and dropping shelljs as a dependency, if > it was only ever used for .exec). > > Any objections? > > Braden >
Re: Android platform scripts
Do it! On Fri, Sep 27, 2013 at 8:13 AM, Braden Shepherdson wrote: > The Android platform scripts use shelljs.exec's synchronous mode. This is a > terrible hack that leaks filehandles by the hundred, wastes lots of CPU > cycles, and can cause EMFILE on OSX because it runs out of filehandles. > > I wanted to rewrite the scripts to be async and use child_process.exec or > .spawn. I started to, but rapidly found that the tangle of callbacks that > resulted was terrible and confusing. > > I propose using Q.js here as well (and dropping shelljs as a dependency, if > it was only ever used for .exec). > > Any objections? > > Braden
Android platform scripts
The Android platform scripts use shelljs.exec's synchronous mode. This is a terrible hack that leaks filehandles by the hundred, wastes lots of CPU cycles, and can cause EMFILE on OSX because it runs out of filehandles. I wanted to rewrite the scripts to be async and use child_process.exec or .spawn. I started to, but rapidly found that the tangle of callbacks that resulted was terrible and confusing. I propose using Q.js here as well (and dropping shelljs as a dependency, if it was only ever used for .exec). Any objections? Braden