This revision was automatically updated to reflect the committed changes.
Closed by commit R824:5e4203cd323a: baloo-widgets: Emit metaDataRequestFinished
once per request (authored by michaelh).
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
ngraham accepted this revision.
REPOSITORY
R824 Baloo Widgets
BRANCH
signalavailable (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham, #dolphin, #frameworks
Cc: dhaumann
elvisangelaccio accepted this revision as: elvisangelaccio.
This revision is now accepted and ready to land.
REPOSITORY
R824 Baloo Widgets
BRANCH
signalavailable (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda,
michaelh marked 3 inline comments as done.
REPOSITORY
R824 Baloo Widgets
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham, #dolphin, #frameworks
Cc: dhaumann
michaelh added inline comments.
INLINE COMMENTS
> filemetadatawidget.cpp:119
> +slotDataAvailable();
> +emit q->metaDataRequestFinished(m_provider->items());
> +}
test with empty `items()`: This code is reached, but the `moc` doesn't signal.
REPOSITORY
R824 Baloo Widgets
REVISION
michaelh updated this revision to Diff 26222.
michaelh added a comment.
Change commit message
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26218=26222
BRANCH
signalavailable (branched from master)
REVISION DETAIL
michaelh added a comment.
Both test for empty failed because the spy timed out!
IMO they should not, because consumers rely on the `metaDataRequestFinished`
signal.
Please tell me if you disagree.
I don't know why they fail, yet. Because line 371 is reached in
michaelh updated this revision to Diff 26218.
michaelh added a comment.
Added test for empty file list
Added test for file list with empty url
Simplified path construction
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26183=26218
michaelh added a comment.
Changed description
Or did you mean the commit messages of 1e3aa22d15d8 and 0f362a3e0de0?
Also, I've committed the accepted revisions to master not Applications/17.1.
Correct?
REPOSITORY
R824 Baloo Widgets
REVISION DETAIL
https://phabricator.kde.org/D10113
michaelh edited the summary of this revision.
REPOSITORY
R824 Baloo Widgets
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham, #dolphin, #frameworks
Cc: dhaumann
elvisangelaccio added inline comments.
INLINE COMMENTS
> michaelh wrote in filemetadataprovider.cpp:323
> In that case it might be safer to do
>
> IndexedDataRetriever *ret = new IndexedDataRetriever(filePath, this);
> ​connect(ret, SIGNAL(finished(KJob*)), this,
>
michaelh updated this revision to Diff 26183.
michaelh added a comment.
Merge branch 'master' of git://anongit.kde.org/baloo-widgets
Change execution order in setFileItems
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26149=26183
BRANCH
michaelh planned changes to this revision.
michaelh added a comment.
Needs rebasing
REPOSITORY
R824 Baloo Widgets
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham, #dolphin, #frameworks
Cc: dhaumann
michaelh updated this revision to Diff 26149.
michaelh added a comment.
Change execution order in setFileItem
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26135=26149
BRANCH
dataavail
REVISION DETAIL
michaelh updated this revision to Diff 26135.
michaelh added a comment.
Correct yet another update error
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26131=26135
BRANCH
dataavail
REVISION DETAIL
https://phabricator.kde.org/D10113
michaelh updated this revision to Diff 26131.
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26129=26131
BRANCH
dataavail
REVISION DETAIL
https://phabricator.kde.org/D10113
AFFECTED FILES
src/filemetadataprovider.cpp
michaelh updated this revision to Diff 26129.
michaelh added a comment.
Add Q_ASSERT
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26120=26129
BRANCH
dataavail
REVISION DETAIL
https://phabricator.kde.org/D10113
AFFECTED FILES
michaelh updated this revision to Diff 26120.
michaelh added a comment.
Revert incorrect diff update
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26117=26120
BRANCH
dataavail
REVISION DETAIL
https://phabricator.kde.org/D10113
michaelh updated this revision to Diff 26117.
michaelh added a comment.
Apply suggested changes
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=26071=26117
BRANCH
unittest
REVISION DETAIL
https://phabricator.kde.org/D10113
AFFECTED
michaelh added inline comments.
INLINE COMMENTS
> elvisangelaccio wrote in filemetadataprovider.cpp:323
> I don't think so, because `IndexedDataRetriever::start()` does not block.
In that case it might be safer to do
IndexedDataRetriever *ret = new IndexedDataRetriever(filePath,
elvisangelaccio added a comment.
+1, looks good to me.
INLINE COMMENTS
> michaelh wrote in filemetadataprovider.cpp:323
> Is it guaranteed, that this is emitted before IndexedDataRetriever job is
> finished?
I don't think so, because `IndexedDataRetriever::start()` does not block.
michaelh marked an inline comment as done.
michaelh added inline comments.
INLINE COMMENTS
> filemetadataprovider.cpp:323
> +emit dataAvailable();
>
> } else {
Is it guaranteed, that this is emitted before IndexedDataRetriever job is
finished?
REPOSITORY
R824 Baloo
michaelh updated this revision to Diff 26071.
michaelh added a comment.
Update signal descriptions
REPOSITORY
R824 Baloo Widgets
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10113?vs=25962=26071
BRANCH
dataavail
REVISION DETAIL
https://phabricator.kde.org/D10113
AFFECTED
dhaumann added inline comments.
INLINE COMMENTS
> filemetadataprovider.h:114
> void loadingFinished();
> +void dataAvailable();
>
Could you add API documentation there what this signal indicates and when to
use it? Typically, if you find code that is documented it is good practice
michaelh retitled this revision from "Emit metaDataRequestFinished once per
request" to "baloo-widgets: Emit metaDataRequestFinished once per request".
REPOSITORY
R824 Baloo Widgets
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham,
michaelh added a dependent revision: D10119: Create test to assert
metaDataRequestFinished is emitted once only.
REPOSITORY
R824 Baloo Widgets
REVISION DETAIL
https://phabricator.kde.org/D10113
To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham, #dolphin, #frameworks
26 matches
Mail list logo
