[Merge] lp:~ken-vandine/content-hub/lp1429695 into lp:content-hub

Michael Sheldon michael.sheldon at canonical.com
Tue Mar 31 13:27:00 UTC 2015


Review: Approve

Approved, but with a note on one of the comments that might be a typo?


Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?

 * Yes

Did CI run pass? If not, please explain why.

 * Yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?

 * Yes

Diff comments:

> === modified file 'import/Ubuntu/Content/contentpeer.cpp'
> --- import/Ubuntu/Content/contentpeer.cpp	2014-08-04 17:57:26 +0000
> +++ import/Ubuntu/Content/contentpeer.cpp	2015-03-30 16:20:53 +0000
> @@ -221,7 +221,6 @@
>  ContentTransfer *ContentPeer::request(ContentStore *store)
>  {
>      TRACE() << Q_FUNC_INFO;
> -
>      ContentHub *contentHub = ContentHub::instance();
>      ContentTransfer *qmlTransfer = NULL;
>      if(m_handler == ContentHandler::Source) {
> @@ -239,8 +238,15 @@
>      }
>      
>      /* We only need to start it for import requests */
> -    if (m_handler == ContentHandler::Source)
> -        qmlTransfer->start();
> +    if (m_handler == ContentHandler::Source) {
> +        // If there peer isn't known, don't request content

Should "there" be "the"?

> +        if (m_peer == cuc::Peer::unknown()) {
> +            qWarning() << "Unknown source for type:" << m_contentType;
> +            qmlTransfer->transfer()->abort();
> +        } else {
> +            qmlTransfer->start();
> +        }
> +    }
>  
>      return qmlTransfer;
>  }
> 


-- 
https://code.launchpad.net/~ken-vandine/content-hub/lp1429695/+merge/254599
Your team Ubuntu Phablet Team is subscribed to branch lp:content-hub.



More information about the Ubuntu-reviews mailing list