Differential D8366

Factoring out lists of url data within KFilePlacesModelTest
ClosedPublic
Actions

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

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

Lint
Lint Skipped
Unit
Unit Tests Skipped
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
mlaurent accepted this revision.Oct 25 2017, 9:46 AM
mlaurent added a subscriber: mlaurent.

seems ok for me

This revision is now accepted and ready to land.Oct 25 2017, 9:46 AM
franckarrecot updated this revision to Diff 21289.Oct 25 2017, 9:54 AM

switch removable after regular devices

ervin requested changes to this revision.Oct 31 2017, 11:27 AM
ervin added a subscriber: ervin.
ervin added inline comments.
autotests/kfileplacesmodeltest.cpp
177

Typo: initial

182

ditto

187

ditto

This revision now requires changes to proceed.Oct 31 2017, 11:27 AM
mlaurent commandeered this revision.Oct 31 2017, 2:46 PM
mlaurent edited reviewers, added: franckarrecot; removed: mlaurent.

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

mlaurent updated this revision to Diff 21750.Nov 2 2017, 9:06 AM

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
ngraham added a comment.Nov 2 2017, 12:55 PM

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

renatoo added a comment.Nov 2 2017, 1:10 PM
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.
franckarrecot updated this revision to Diff 22900.Nov 24 2017, 2:30 PM

rebased over in review renato 's patches

ngraham added a comment.Nov 26 2017, 3:59 AM

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

anthonyfieroni added a subscriber: anthonyfieroni.Nov 27 2017, 8:22 AM

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

franckarrecot added a comment.Nov 27 2017, 11:19 AM

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.

franckarrecot added a comment.Nov 27 2017, 2:21 PM

this one is landable now that its dependencies are pushed.

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 ↗(On Diff #21646)

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

PathPackages
Mautotests/kfileplacesmodeltest.cpp (92 lines)

Diff 21750

View Options

autotests/kfileplacesmodeltest.cpp

Loading...