Code review comment for lp:~qqworini/ubuntu-rssreader-app/add-demo-to-trunk

Revision history for this message
Svenn-Arne Dragly (dragly) wrote :

Hi Roman,

What functions are not working as you'd expect? Is it the functions in
the databasemodule.js file? If so, that is likely because I tried to
change the functions so that they return the results rather than set
the global dbResult. The global object could potentially cause some
trouble with for instance nested loops. It is of course possible that I
might have broken something in the process.

In any case, if you pinpoint the functions that are not working for you
I'd be happy to fix them. If you think the move from a global dbResult
object is a bad idea, I could also revert all changes I've made in
databasemodule.js.

Which would you prefer? That I revert all the changes in
databasemodule.js or that I fix the specific functions that cause
trouble?

I'll hopefully have time to see to it tomorrow.

Cheers,
Svenn-Arne

On Thu 18 Apr 2013 10:31:24 PM CEST, Roman Shchekin wrote:
> Hi all!
>
> Svenn, my little fix of your version merged, it's ok. But I haven't seen
> that not all functions of my old version (Revision of trunk) works well.
> You can fix this? I mean apply your features (like TodayPage) without
> changing my previous version?
>
> Cheers,
> Roman.
>
>
> 2013/4/18 <email address hidden>
>
>> The proposal to merge lp:~qqworini/ubuntu-rssreader-app/add-demo-to-trunk
>> into lp:ubuntu-rssreader-app has been updated.
>>
>> Status: Approved => Merged
>>
>> For more details, see:
>>
>> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/add-demo-to-trunk/+merge/159325
>> --
>>
>> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/add-demo-to-trunk/+merge/159325
>> You are reviewing the proposed merge of
>> lp:~qqworini/ubuntu-rssreader-app/add-demo-to-trunk into
>> lp:ubuntu-rssreader-app.
>>
>

« Back to merge proposal