[Merge] lp:~michael-sheldon/content-hub/debian-peers into lp:content-hub

Renato Araujo Oliveira Filho renato.filho at canonical.com
Thu Jun 26 14:42:05 UTC 2014


Whats happen if it finds the same app more than once.

IMO the priority should be: (first has more priority)

~/.local/usr/share/content-hub/peers/
/usr/local/share/content-hub/peers/
/usr/share/content-hub/peers/





Diff comments:

> === modified file 'src/com/ubuntu/content/service/hook.cpp'
> --- src/com/ubuntu/content/service/hook.cpp	2014-06-04 16:50:21 +0000
> +++ src/com/ubuntu/content/service/hook.cpp	2014-06-26 14:36:31 +0000
> @@ -23,6 +23,7 @@
>  #include <QDir>
>  #include <QStandardPaths>
>  #include <QTimer>
> +#include <QVector>
>  #include <com/ubuntu/content/peer.h>
>  
>  #include "debug.h"
> @@ -66,13 +67,15 @@
>       * no JSON file installed in this path.
>       */
>  
> -    QDir contentDir(
> +    QVector<QDir> contentDirs;
> +
> +    contentDirs.append(QDir(
>          QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation)
>          + QString("/")
> -        + QString("content-hub"));
> -
> -    if (not contentDir.exists())
> -        return_error();
> +        + QString("content-hub")));
> +    
> +    contentDirs.append(QDir("/usr/share/content-hub/peers/"));
> +    contentDirs.append(QDir("/usr/share/local/content-hub/peers/"));

IMO /usr/share/local/content-hub/peers/ should came first than /usr/share/content-hub/peers/.

>  
>      QStringList all_peers;
>      registry->enumerate_known_peers([&all_peers](const com::ubuntu::content::Peer& peer)
> @@ -83,13 +86,36 @@
>      Q_FOREACH(QString p, all_peers)
>      {
>          TRACE() << Q_FUNC_INFO << "Looking for" << p;
> -        QStringList pp = contentDir.entryList(QStringList("*"+ p));
> -        if (pp.isEmpty())
> +        bool foundPeer = false;
> +        Q_FOREACH(QDir contentDir, contentDirs)
> +        {
> +            QStringList pp = contentDir.entryList(QStringList("*"+ p));
> +            if (!pp.isEmpty()) {
> +                foundPeer = true;
> +            }
> +        }
> +        if(!foundPeer) {
>              registry->remove_peer(com::ubuntu::content::Peer{p});
> -    }
> -
> -    Q_FOREACH(QFileInfo f, contentDir.entryInfoList(QDir::Files))
> -        add_peer(f);
> +        }
> +    }
> +
> +    bool peerDirsExist = false;
> +
> +    Q_FOREACH(QDir contentDir, contentDirs)
> +    {
> +
> +        if (contentDir.exists()) 
> +        {
> +            peerDirsExist = true;
> +
> +            Q_FOREACH(QFileInfo f, contentDir.entryInfoList(QDir::Files))
> +                add_peer(f);
> +        }
> +
> +    }
> +
> +    if(!peerDirsExist)
> +        return_error("No peer setting directories exist.");
>  
>      deleteLater();
>      QCoreApplication::instance()->quit();
> 


-- 
https://code.launchpad.net/~michael-sheldon/content-hub/debian-peers/+merge/224639
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~michael-sheldon/content-hub/debian-peers into lp:content-hub.



More information about the Ubuntu-reviews mailing list