cordova-dev mailing list archives

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