Code review comment for lp:~michihenning/storage-framework/qml-improvements

Revision history for this message
James Henstridge (jamesh) wrote :

This all looks a bit messed up.

In AccountImpl, we have:

     static Account make_account(std::shared_ptr<RuntimeImpl> const& runtime_impl,
                                 QString const& bus_name,
                                 QString const& object_path,
                                 QString const& id,
                                 QString const& service_id,
                                 QString const& display_name);

Which is being invoked in AccountJobImpl as:

            accounts_.append(AccountImpl::make_account(runtime,
                                                       bus_name,
                                                       object_path,
                                                       a->serviceId(),
                                                       "",
                                                       a->displayName()));

That is, it is setting id=a->serviceId(), service_id="", display_name=a->displayName(). This would explain my confusion when discussing this with you yesterday.

This looks like it is a transposition of the 4th and 5th arguments relative to the v1 API. So before merging this, it'd be good to decide what information exactly we're actually trying expose here.

review: Needs Information

« Back to merge proposal