cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Stewart <rstew...@adobe.com>
Subject Re: Major refactoring of Plugman and CLI
Date Tue, 24 Sep 2013 20:22:22 GMT
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" <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
View raw message