[cosmo-dev] Putting the A back in Ajax

[Ted Leung]

So I know that this is mostly water under the bridge by now, but I  
did want to comment.    In the Cosmo project, we have a culture of  
"commit then review", and I am very happy with how that has been  
working for us.    However, that doesn't mean that a code-review  
before a commit is a bad idea or out of our process.   In fact, on  
several occasions we've done just that.    It's true that we  
generally do not do a "conventional" code review, where we all get  
into a room with printed copies of the code and walk through it  
together line by line.   The key principle that should be in  
operation here is a foundation of open source projects - "many eyes  
make all bugs shallow".   Sometimes it's good to have the eyes look  
before pressing return, sometimes it's good to have the eyes look  
after return got pressed.

Ted

On Nov 14, 2007, at 10:36 AM, Travis Vachon wrote:

> Could you clarify what you mean by formal code review Mikeal? AFAIK  
> we don't have any process or precedent for this. Please correct me  
> if I'm wrong!
>
> -Travis
>
> On Nov 14, 2007, at 10:25 AM, Mikeal Rogers wrote:
>
>> Could we have a formal code review of this work with mde.
>>
>> I'm just guessing here, but I bet there is a lot of code in the UI  
>> that assumes these operations block that will need to get  
>> refactored for async. According to some earlier emails to the list  
>> by you, understanding a lot of that code "as a whole" is a bit  
>> difficult.
>>
>> +1 tho, this definitely needs to happen.
>>
>> -Mikeal
>>
>> On Nov 14, 2007, at November 14, 20079:42 AM, Travis Vachon wrote:
>>
>>> Hi folks
>>>
>>> The recent problem on hub highlighted a flaw in our client code,  
>>> namely our use of synchronous XHR calls. A grep for the parameter  
>>> that makes this happen looks like:
>>>
>>> ~/Development/osaf/cosmo/git/cosmo/src/main/webapp/js/cosmo% grep  
>>> -r -e "sync: \?true" *
>>> app/pim.js:               var collection = this.serv.getCollection 
>>> (params.collectionUrl, {sync:true}).results[0];
>>> app/pim.js:            var userCollections =  
>>> this.serv.getCollections({sync: true}).results[0];
>>> app/pim.js:            var subscriptions =  
>>> this.serv.getSubscriptions({sync:true}).results[0];
>>> service/conduits/ 
>>> common.js:                                                           
>>>     {sync: true}).results[0];
>>> service/conduits/ 
>>> common.js:                                                           
>>>     {sync: true, noAuth: true}).results[0];
>>> ui/imagegrid.js:    sync: true,
>>> view/cal/common.js:                        { start: start, end:  
>>> end }, { sync: true });
>>> view/list/canvas.js:                { sync: true })];
>>> view/list/canvas.js:                  { sync: true }));
>>> view/list/common.js:            { sync: true });
>>> view/service.js:                var deferred =  
>>> cosmo.app.pim.serv.getCollection 
>>> (cosmo.app.pim.currentCollection.getUrls().self, {sync:true});
>>>
>>>
>>> As you can see, we're using synchronous calls quite a bit.  This  
>>> means that while the browser is waiting for the server to return  
>>> data the entire program hangs, something you probably noticed if  
>>> you signed into hub yesterday. The solution is to convert all  
>>> these pieces of code to a callback based model and make the XHR  
>>> calls asynchronously.
>>>
>>> I'd be happy to make this happen, but wanted to ping the list for  
>>> comments and ideas on potential gotchas.
>>>
>>> Thoughts?
>>>
>>> -Travis
>>> _______________________________________________
>>> cosmo-dev mailing list
>>> cosmo-dev at lists.osafoundation.org
>>> http://lists.osafoundation.org/mailman/listinfo/cosmo-dev
>>
>> _______________________________________________
>> cosmo-dev mailing list
>> cosmo-dev at lists.osafoundation.org
>> http://lists.osafoundation.org/mailman/listinfo/cosmo-dev
>
> _______________________________________________
> cosmo-dev mailing list
> cosmo-dev at lists.osafoundation.org
> http://lists.osafoundation.org/mailman/listinfo/cosmo-dev