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

Revision history for this message
Xavi Garcia (xavi-garcia-mena) wrote :

As discussed in the standup, a couple of things I saw taking a quick look:

The following signals:
    void statusChanged(Status status) const;
    void error(StorageError const& e) const;
    void finished(Uploader const& uploader) const;
Can be found on the Uploader and UploadJob.

It's a bit confusing if we need to connect to both, or just connection to the one in the Job or the Uploader is just fine.

And the second one is about the signal declaration itself.
I had issues related to this a few weeks ago.

For example the signal:

enum class Status { Loading, Finished, Error };
void statusChanged(Status status) const;

should be:
void statusChanged(UploadJob::Status status) const;

and I should verify, but maybe Qt needs it to be:
void statusChanged(unity::storage::qt::UploadJob::Status status) const;

if not, when creating the moc code it does not understand the enum and we cannot access it from QML or, for example QSignalSpy. In fact the issue I've got was with QSignalSpy, when trying to convert from QVariant to the enum class.

« Back to merge proposal