cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesse <purplecabb...@gmail.com>
Subject Re: Major refactoring of Plugman and CLI
Date Tue, 24 Sep 2013 20:18:44 GMT
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 <purplecabbage@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 <braden@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 <shazron@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 <
> purplecabbage@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 <braden@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.kadri@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 <
> >>> braden@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 <
> >>>>> braden@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
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message