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 Wed, 25 Sep 2013 14:11:57 GMT
Confirming that this is not part of the 3.1.0 release. We want it to bake a
little longer first. I've been using it recently and it's been working fine
for me, but there may be subtle bugs lurking in so large a refactoring with
no type system to back me up.

Braden


On Tue, Sep 24, 2013 at 5:50 PM, Jesse <purplecabbage@gmail.com> wrote:

> 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 <rstewart@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" <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