D13813: Make this test work again with new uds implementation

bcooksley added a comment. That would have happened locally, shouldn't be anything to do with our systems - especially given it's a Travis CI name/address being mentioned. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks,

D13813: Make this test work again with new uds implementation

elvisangelaccio added a comment. In D13813#310067 , @jtamate wrote: > In D13813#310047 , @cfeck wrote: > > > Authored by a Bot? > > > Somehow my name and email address in .gitconfig got

D13813: Make this test work again with new uds implementation

jtamate added a comment. In D13813#310047 , @cfeck wrote: > Authored by a Bot? Somehow my name and email address in .gitconfig got changed. How? No idea. Should revert and push again with the right name? REPOSITORY R318 Dolphin

D13813: Make this test work again with new uds implementation

elvisangelaccio added subscribers: nalvarez, bshah, bcooksley. elvisangelaccio added a comment. In D13813#310047 , @cfeck wrote: > Authored by a Bot? Yeah I was about to ask the same question. @bshah @bcooksley @nalvarez ?

D13813: Make this test work again with new uds implementation

cfeck added a comment. Authored by a Bot? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks, elvisangelaccio Cc: cfeck, dfaure, aacid, ngraham, bruns, elvisangelaccio, kfm-devel, spoorun, navarromorales, firef, andrebarros,

D13813: Make this test work again with new uds implementation

This revision was automatically updated to reflect the committed changes. Closed by commit R318:2104d18a6760: Make this test work again with new uds implementation (authored by Travis CI Bot tra...@travis-ci.org). REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE

D13813: Make this test work again with new uds implementation

elvisangelaccio accepted this revision. elvisangelaccio added a comment. This revision is now accepted and ready to land. Right. Then let's ship it as is for now, thanks! REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks,

D13813: Make this test work again with new uds implementation

jtamate added a comment. In D13813#309371 , @elvisangelaccio wrote: > @jtamate Any updates on this? Can you use the new `fastInsert()` calls here? Not until dolphin depends on KIO 5.47, currently its minimum required kio version is

D13813: Make this test work again with new uds implementation

elvisangelaccio added a comment. @jtamate Any updates on this? Can you use the new `fastInsert()` calls here? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks, elvisangelaccio Cc: dfaure, aacid, ngraham, bruns,

D13813: Make this test work again with new uds implementation

dfaure added a comment. In D13813#287032 , @jtamate wrote: > > But ForwardingSlaveBase::prepareUDSEntry in KIO itself, uses insert() to replace UDS_URL, and in fact all of ForwardingSlaveBase should use replace(), as well as users of

D13813: Make this test work again with new uds implementation

jtamate added a comment. > But ForwardingSlaveBase::prepareUDSEntry in KIO itself, uses insert() to replace UDS_URL, and in fact all of ForwardingSlaveBase should use replace(), as well as users of ForwardingSlaveBase like kio_desktop. In ForwardingSlaveBase::prepareUDSEntry, it uses

D13813: Make this test work again with new uds implementation

dfaure added a comment. Albert, you are of course fully right. My reasoning was "in practice, [outside of strangely written tests such as the one here], there is zero reason to replace entries in a UDSEntry. All kioslaves either create a new UDSEntry for every file, or do reuse it after

D13813: Make this test work again with new uds implementation

aacid added a subscriber: dfaure. aacid added a comment. In D13813#286358 , @jtamate wrote: > In D13813#286313 , @aacid wrote: > > > Did we break how KIO works and we're now going after the users of

D13813: Make this test work again with new uds implementation

jtamate updated this revision to Diff 37119. jtamate retitled this revision from "make this test work again with new uds implementation" to "Make this test work again with new uds implementation". jtamate edited the summary of this revision. jtamate added a comment. Call items.append inside

D13813: make this test work again with new uds implementation

bruns added inline comments. INLINE COMMENTS > kfileitemmodeltest.cpp:1562 > +entry[i].insert(KIO::UDSEntry::UDS_ACCESS_TIME, 0); > +} > + if you move the `insert(UDS_NAME, ...)` and `insert(UDS_USER, ...)` in front of the loop, you can call `items.append(entry[i]...)` inside the

D13813: make this test work again with new uds implementation

jtamate added a comment. In D13813#286313 , @aacid wrote: > Did we break how KIO works and we're now going after the users of KIO and fixing them or was this a "bad usage" from starters that worked by chance? We break how KIO works and

D13813: make this test work again with new uds implementation

aacid added a comment. Did we break how KIO works and we're now going after the users of KIO and fixing them or was this a "bad usage" from starters that worked by chance? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks,

D13813: make this test work again with new uds implementation

ngraham added a comment. FWIW, I'd be okay with bumping the minimum KIO version to 5.47 in master. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks, elvisangelaccio Cc: ngraham, bruns, elvisangelaccio, kfm-devel, spoorun,

D13813: make this test work again with new uds implementation

elvisangelaccio requested changes to this revision. elvisangelaccio added a comment. This revision now requires changes to proceed. In D13813#285441 , @jtamate wrote: > Updated the patch with context. > > It will not work with kio previous to

D13813: make this test work again with new uds implementation

jtamate updated this revision to Diff 36976. jtamate edited the summary of this revision. jtamate added a comment. Use 3 entries instead of replace(). Now it will work with any kio version. REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE

D13813: make this test work again with new uds implementation

jtamate updated this revision to Diff 36975. jtamate added a comment. Updated the patch with context. It will not work with kio previous to 5.47, unless there is a way to check the kio version at compile time or runtime. REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE

D13813: make this test work again with new uds implementation

bruns added a comment. @jtamate , can you please use arc diff to upload patches - arc sucks, but having no patch context sucks even more ... REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks Cc: bruns, elvisangelaccio,

D13813: make this test work again with new uds implementation

elvisangelaccio added a comment. Will it still work if we link against an old kio version? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks Cc: elvisangelaccio, kfm-devel, spoorun, navarromorales, isidorov, firef,

D13813: make this test work again with new uds implementation

jtamate added a dependent revision: D13814: Speedup sort. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D13813 To: jtamate, #dolphin, #frameworks Cc: kfm-devel, spoorun, navarromorales, isidorov, firef, andrebarros, emmanuelp

D13813: make this test work again with new uds implementation

jtamate created this revision. jtamate added reviewers: Dolphin, Frameworks. Restricted Application added a project: Dolphin. Restricted Application added a subscriber: kfm-devel. jtamate requested review of this revision. REVISION SUMMARY With the new uds implementation, when some data is