cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Braden Shepherdson <bra...@chromium.org>
Subject Re: Major refactoring of Plugman and CLI
Date Tue, 24 Sep 2013 20:22:24 GMT
I guess I wasn't clear in my email, but this is exactly the API that
**didn't** change. I did more or less what you suggested to support it.

You should still be able to make that call and it will work fine. If it's
not, then something has gone wrong and we'll fix it.

Braden


On Tue, Sep 24, 2013 at 4:18 PM, Jesse <purplecabbage@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 <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