unity-scopes-api

Merge lp://staging/~stolowski/unity-scopes-api/sss-no-wait-for-all into lp://staging/unity-scopes-api/devel

sss-no-wait-for-all
Merge into devel

Proposed by Paweł Stołowski on 2014-08-28

Status:

Merged

Approved by:

Marcus Tomlinson on 2014-09-03

Approved revision:

476

Merged at revision:

474

Proposed branch:

lp://staging/~stolowski/unity-scopes-api/sss-no-wait-for-all

Merge into:

lp://staging/unity-scopes-api/devel

Diff against target:

1524 lines (+550/-427)

11 files modified

include/unity/scopes/internal/smartscopes/HttpClientInterface.h (+7/-5)
include/unity/scopes/internal/smartscopes/HttpClientQt.h (+4/-4)
include/unity/scopes/internal/smartscopes/HttpClientQtThread.h (+3/-1)
include/unity/scopes/internal/smartscopes/SmartScopesClient.h (+26/-9)
src/scopes/internal/smartscopes/HttpClientQt.cpp (+10/-13)
src/scopes/internal/smartscopes/HttpClientQtThread.cpp (+36/-6)
src/scopes/internal/smartscopes/SmartScope.cpp (+121/-96)
src/scopes/internal/smartscopes/SmartScopesClient.cpp (+171/-177)
test/gtest/scopes/internal/smartscopes/HttpClient/HttpClient_test.cpp (+25/-23)
test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp (+101/-52)
test/gtest/scopes/internal/smartscopes/smartscopesproxy/smartscopesproxy_test.cpp (+46/-41)

To merge this branch:

bzr merge lp://staging/~stolowski/unity-scopes-api/sss-no-wait-for-all

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2014-09-03
Marcus Tomlinson (community)		2014-09-01	Approve on 2014-09-03
Michi Henning (community)		2014-08-28	Needs Information on 2014-09-02
Review via email: mp+232586@code.staging.launchpad.net

Commit message

Push smart scopes online results immediately as they become available.

Description of the change

Push smart scopes online results immediately as they become available.
This required quite a bit of refactoring. Still WIP.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-29:

PASSED: Continuous integration, rev:474
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/1144/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/702
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/697
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/697/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/696

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/1144/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michi Henning (michihenning) wrote on 2014-09-02:

Minor comment:

std::future<void> HttpClientQt::HttpSession::get_future()
{
return promise_->get_future();
}

Not totally sure here, but it looks a little strange. Why is promise_ a shared_ptr instead of a promise? Couldn't it just be a plain promise?

Marcus, you are most familiar with this code, could you cast your eye over it too please?

review: Needs Information

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2014-09-02:

> Minor comment:
>
> std::future<void> HttpClientQt::HttpSession::get_future()
> {
> return promise_->get_future();
> }
>
> Not totally sure here, but it looks a little strange. Why is promise_ a
> shared_ptr instead of a promise? Couldn't it just be a plain promise?
>
> Marcus, you are most familiar with this code, could you cast your eye over it
> too please?

Hmmm, no idea why this was a shared_ptr. Must have been some legacy reason that I removed since. Pawel, could you change the promise_ member to a std::promise<std::string> for me please? Sorry about that :/

lp://staging/~stolowski/unity-scopes-api/sss-no-wait-for-all updated on 2014-09-02

475. By Paweł Stołowski on 2014-09-02: Don't use shared ptr for promise member.

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2014-09-02:

> > Minor comment:
> >
> > std::future<void> HttpClientQt::HttpSession::get_future()
> > {
> > return promise_->get_future();
> > }
> >
> > Not totally sure here, but it looks a little strange. Why is promise_ a
> > shared_ptr instead of a promise? Couldn't it just be a plain promise?
> >
> > Marcus, you are most familiar with this code, could you cast your eye over
> it
> > too please?
>
> Hmmm, no idea why this was a shared_ptr. Must have been some legacy reason
> that I removed since. Pawel, could you change the promise_ member to a
> std::promise<std::string> for me please? Sorry about that :/