Oops, I missed that Braden, thanks. @Ryan, the code is committed, but won't be pushed to npm for at least a week. We will have a chance to run it through the rigors first, although it is seeming like there will be little to no impact.
@purplecabbage risingj.com On Tue, Sep 24, 2013 at 1:22 PM, Ryan Stewart <rstew...@adobe.com> wrote: > Apologies as I'm having a little bit of trouble following this > conversation. Is this change already in Cordova CLI? Is it part of the > 3.1.0 release? > > =Ryan > > On 9/24/13 1:18 PM, "Jesse" <purplecabb...@gmail.com> wrote: > > >So, in the phonegap-cli, which uses cordova-cli, we have this call: > > > >var cordova = require('cordova'); > >... > >cordova.create(options.path, options.id, options.name, function(e) { ... > > > > > >So this is definitely broken now as a result of this change, and has to be > >re-written to support promises. > > > >Couldn't you have maintained support for callbacks, by doing this? > > > >module.exports = function create (dir, id, name, callback) { > >var retQ = Q(); > > if(callback) { > > // output callback deprecation notice > > retQ.then(callback); > > } > > return retQ; > >} > > > >Not that I like lingering potentially unused functionality, but short of > >knowing ahead that this is going to change, it is hard to prepare for. > > > > > > > > > >@purplecabbage > >risingj.com > > > > > >On Tue, Sep 24, 2013 at 12:21 PM, purplecabbage > ><purplecabb...@gmail.com>wrote: > > > >> Thanks for clarifying Braden. > >> My general rule is if I am changing many files, I will run it by the > >>list. > >> Also if I am touching other platforms, I will always consult the list. > >> Luckily/unluckily, many of my drastic changes have happened in the > >>win/wp > >> area where there are far fewer hands in the pot. > >> > >> Sent from my iPhone > >> > >> > On Sep 24, 2013, at 11:49 AM, Braden Shepherdson <bra...@chromium.org > > > >> wrote: > >> > > >> > I get the impression that "talking about it internally" came out > >>wrong. > >> > Here's what actually happened: > >> > > >> > - I started out to fix some bugs, including killing use of shell.exec > >> > because it leaks filehandles and sometimes causes fatal errors. > >> > - I adopted promises to control the horribly deep callback trees we > >>were > >> > building up. > >> > - That improved the code so much that it spread to the rest of the > >>tools > >> > and became a large refactoring. > >> > - I was worried that I now needed to bring this up with the list and > >>get > >> > buy-in, since it had become bigger than fixing a few bugs. > >> > - On consulting with the others who happened to be in the room at the > >> time, > >> > the feeling was that since the outside API hadn't changed, this was > >>fine. > >> > Bug fixes and code cleanup don't need buy-in from everyone. However, I > >> > should definitely announce the change. > >> > - Here we are. > >> > > >> > I guess I haven't found the balance of what should be done silently, > >>what > >> > announced as it goes out, and what discussed ahead of time. Since the > >> > feeling is that this should have been mentioned in advance, I will > >>adjust > >> > in the future. > >> > > >> > Braden > >> > > >> >> On Tue, Sep 24, 2013 at 2:20 PM, Shazron <shaz...@gmail.com> wrote: > >> >> > >> >> Definitely. Talking about changes is cool, not talking about it -- > >>not > >> >> cool. > >> >> > >> >> > >> >> On Tue, Sep 24, 2013 at 11:01 AM, purplecabbage < > >> purplecabb...@gmail.com > >> >>> wrote: > >> >> > >> >>> Q is a good choice. Not talking about it was absolutely the wrong > >> choice. > >> >>> There are no internal teams , there is only this list. > >> >>> > >> >>> Sent from my iPhone > >> >>> > >> >>>>> On Sep 24, 2013, at 6:48 AM, Braden Shepherdson > >><bra...@chromium.org > >> > > >> >>>> wrote: > >> >>>> > >> >>>> We debated internally at Google how much to talk about this. In the > >> end > >> >>> we > >> >>>> decided that since the external APIs were not changing, this could > >>be > >> >>>> claimed as an internal refactoring. I'm not sure whether that was > >>the > >> >>> right > >> >>>> call. > >> >>>> > >> >>>> About fetch and platforms, to be clear, those are far from the only > >> >>> modules > >> >>>> that have changes, they're just the examples I chose. Using almost > >>any > >> >> of > >> >>>> the internal modules directly will require refactoring. > >> >>>> > >> >>>> Braden > >> >>>> > >> >>>> > >> >>>>> On Tue, Sep 24, 2013 at 6:56 AM, Anis KADRI <anis.ka...@gmail.com > > > >> >>> wrote: > >> >>>>> > >> >>>>> cool. > >> >>>>> > >> >>>>> I don't think we're using fetch/platforms directly. > >> >>>>> > >> >>>>> -a > >> >>>>> > >> >>>>>> On Tue, Sep 24, 2013 at 11:35 AM, Brian LeRoux <b...@brian.io> > >>wrote: > >> >>>>>> Kewl. I'm down and happen to really like Q. Not sure everyone > >>will > >> >>> agree. > >> >>>>>> Maybe next time a heads up to the list so we can discuss arch > >> changes > >> >>>>> like > >> >>>>>> this. > >> >>>>>> > >> >>>>>> > >> >>>>>> On Mon, Sep 23, 2013 at 8:13 PM, Braden Shepherdson < > >> >>> bra...@chromium.org > >> >>>>>> wrote: > >> >>>>>> > >> >>>>>>> Whoops, I forgot to mention, I created and pushed a > >>cordova-3.1.x > >> >>>>> branch of > >> >>>>>>> both tools before merging this; fixes for the 3.1.0 release > >>should > >> >> be > >> >>> in > >> >>>>>>> there. I don't intend to launch the refactored code to NPM until > >> >> we've > >> >>>>> had > >> >>>>>>> at least a week of trying it out. > >> >>>>>>> > >> >>>>>>> Braden > >> >>>>>>> > >> >>>>>>> > >> >>>>>>> On Mon, Sep 23, 2013 at 2:08 PM, Braden Shepherdson < > >> >>>>> bra...@chromium.org > >> >>>>>>>> wrote: > >> >>>>>>> > >> >>>>>>>> tl;dr: Plugman and CLI now uses Q.js[1] Promises internally > >> instead > >> >>> of > >> >>>>>>>> callbacks. This has significantly clarified and shortened the > >> code. > >> >>>>> The > >> >>>>>>>> public API (plugman.fetch, cordova.platform, etc.) HAVE NOT > >> >> changed! > >> >>>>>>>> > >> >>>>>>>> If you use CLI on the command line, nothing has changed. > >> >>>>>>>> > >> >>>>>>>> If you downstream CLI and/or Plugman, but use cordova.foo and > >> >>>>>>> plugman.foo, > >> >>>>>>>> nothing has changed (except possibly that a few calls are a bit > >> >> more > >> >>>>>>> async > >> >>>>>>>> than before, so code that cheats and pretends they're sync > >>might > >> >> fail > >> >>>>>>> now). > >> >>>>>>>> > >> >>>>>>>> If you downstream either one, but require internal modules like > >> >>>>> fetch.js > >> >>>>>>>> or platform.js directly, you should stop doing that and use > >> >>>>> plugman.fetch > >> >>>>>>>> etc. instead. If you want to continue calling them directly, > >> you'll > >> >>>>> need > >> >>>>>>> to > >> >>>>>>>> port to use promises. > >> >>>>>>>> > >> >>>>>>>> If you've been working on Plugman or CLI and I just broke > >> >> everything, > >> >>>>>>> feel > >> >>>>>>>> free to yell at me on IRC (#cordova, shepheb) or Gtalk (braden > >>at > >> >>>>> google > >> >>>>>>>> dot com) or email. It's not hard to port things to handle > >>promises > >> >>>>> (see > >> >>>>>>>> below), and their basic use is not hard to understand (see the > >> >>>>>>> tutorial[1]). > >> >>>>>>>> > >> >>>>>>>> If you really do need to port something, and you used to do a > >> >>> function > >> >>>>>>>> call like this: > >> >>>>>>>> > >> >>>>>>>> whateverFunc(args..., function(err){ > >> >>>>>>>> if (err) { > >> >>>>>>>> foo > >> >>>>>>>> } else { > >> >>>>>>>> bar > >> >>>>>>>> } > >> >>>>>>>> }); > >> >>>>>>>> > >> >>>>>>>> the correct call is now: > >> >>>>>>>> > >> >>>>>>>> whateverFunc(args...).done(function() { > >> >>>>>>>> bar > >> >>>>>>>> }, function(err) { > >> >>>>>>>> foo > >> >>>>>>>> }); > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> [1] Q.js tutorial at https://github.com/kriskowal/q > >> >> > >> > >