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 18:49:00 GMT
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