Differential D8366

Factoring out lists of url data within KFilePlacesModelTest
ClosedPublic
Actions

Authored by franckarrecot on Oct 19 2017, 1:03 PM.

Tags

Subscribers

dfaure, anthonyfieroni, ervin and 3 others

Details

Reviewers

renatoo
ervin
mlaurent

Commits

R241:3e537c4dd935: Factoring out lists of url data within KFilePlacesModelTest

Summary

Instead of reusing the same string lists all over the place
we factor them out, easier to read and maintain.

Depends on D8948

Test Plan

It's within a test

Diff Detail

Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

franckarrecot created this revision.Oct 19 2017, 1:03 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptOct 19 2017, 1:03 PM

Restricted Application added a subscriber: Frameworks. · View Herald Transcript

seems ok for me

This revision is now accepted and ready to land.Oct 25 2017, 9:46 AM

switch removable after regular devices

mlaurent mentioned this in D8367: Hidding place groups implementation in KFilePlacesModel.Oct 30 2017, 2:25 PM

mlaurent added a dependent revision: D8367: Hidding place groups implementation in KFilePlacesModel.Oct 30 2017, 2:59 PM

ngraham added a subscriber: ngraham.Oct 30 2017, 3:23 PM

ervin requested changes to this revision.Oct 31 2017, 11:27 AM

ervin added a subscriber: ervin.

ervin added inline comments.

autotests/kfileplacesmodeltest.cpp
188	Typo: initial
193	ditto
198	ditto

This revision now requires changes to proceed.Oct 31 2017, 11:27 AM

impossible to rebase this one...
I will need to rewrite all this patch

Fix typo

Rebase it

ervin accepted this revision.Nov 2 2017, 10:35 AM

This revision is now accepted and ready to land.Nov 2 2017, 10:35 AM

renatoo accepted this revision.Nov 2 2017, 11:47 AM

Is this dependent on any of the other patches floating around?

In D8366#163427, @ngraham wrote:

Is this dependent on any of the other patches floating around?

Probably not, but that will affect (possible conflicts) with all patches

franckarrecot commandeered this revision.Nov 24 2017, 2:30 PM

franckarrecot edited reviewers, added: mlaurent; removed: franckarrecot.

rebased over in review renato 's patches

Is this ready to go in, or do we need some more time for review?

Ship it, it's only tests and looks good.

franckarrecot edited the summary of this revision. (Show Details)Nov 27 2017, 11:13 AM

franckarrecot added a dependency: D8948: Created an auxiliary function 'KFilePlacesModel::movePlace'.

I'll ship it as soon as the commits it's built on will be shipped. Sorry I rebased this one to renato new commit split so we would avoid conflict if interlaced commit pushed happend.

this one is landable now that its dependencies are pushed.

update

Closed by commit R241:3e537c4dd935: Factoring out lists of url data within KFilePlacesModelTest (authored by franckarrecot, committed by ngraham). · Explain WhyNov 27 2017, 2:27 PM

This revision was automatically updated to reflect the committed changes.

dfaure added a subscriber: dfaure.Dec 2 2017, 9:43 AM

dfaure added inline comments.

autotests/kfileplacesmodeltest.cpp
966	Why this change? It created a regression in the CI. Please debug fully. If this depends on your real-user's trash being full or not, then something's broken ;) QStandardPaths' test mode should make it look in a different config dir than ~/.config (where trashrc stores this information). http://ci-logs.kde.flaska.net/0b/0bc40d7c8b71af2c3861f5b39a4ebee91b046371/rebuilddep/rebuilddep-kf5-qt57-clang-el7/27e1666/shell_output.log

Revision Contents
Changeset List

			Path	Packages
M			autotests/kfileplacesmodeltest.cpp (152 lines)

Diff 23027

autotests/kfileplacesmodeltest.cpp

Loading...