Code review comment for lp:~andrewsomething/ubuntu-rssreader-app/sharing

Revision history for this message
Andrew Starr-Bochicchio (andrewsomething) wrote :

Thanks for the review.

On Sun, Sep 1, 2013 at 5:33 AM, Roman Shchekin <email address hidden> wrote:
> Must Ubuntu.OnlineAccounts and Friends modules be manually installed or not?
> I've got errors: "module "Ubuntu.OnlineAccounts" is not installed". Same for "Friends".

My understanding is that they should be part of the default image.
They're going to be used for contact sync and are also used by the
Facebook and Friends apps. Though I don't actually have a device
running Ubuntu Touch, so I don't know if this is a reality yet. I
guess the SDK isn't pulling them in yet. I think the packages are
qtdeclarative5-accounts-plugin and qtdeclarative5-friends-plugin. I'm
running saucy, and they are already present here...

> BTW, I think that:
> property var article: rssListview.model.get(rssListview.currentIndex)
> is bad practice. Better will be set property "article" in PopupUtils.open() method when creating Popover.
>
> And this article.shareAccountId = accts.accountId
> is not pretty clear code too. Use additional property instead of mixing all data in one.

I'll try to pretty this up sometime today. This is my first real step
into the QML world. Is there any kind of widely accepted style guide
akin to Python's PEP8?

Thanks!

-- Andrew Starr-Bochicchio

   Ubuntu Developer <https://launchpad.net/~andrewsomething>
   Debian Developer <http://qa.debian.org/developer.php?login=asb>
   PGP/GPG Key ID: D53FDCB1

« Back to merge proposal