D17302: Add test for adding properties to result

astippich abandoned this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, LeGast00n, sbergeron, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich added a comment. ping! this now tests the resulting document and tests how the different data types are added to it. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, LeGast00n, domson,

D17302: Add test for adding properties to result

astippich added a comment. friendly ping REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich added a comment. ping! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich added a comment. ping REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich updated this revision to Diff 56729. astippich added a comment. - Test result document REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17302?vs=52894=56729 BRANCH test_result REVISION DETAIL https://phabricator.kde.org/D17302 AFFECTED FILES

D17302: Add test for adding properties to result

astippich added a comment. In D17302#439605 , @bruns wrote: > IMHO this test is pointless, as the contents of the variant map is inaccessible, it is only an intermediate storage. The correct way to retrieve the data is via `Result::document()`.

D17302: Add test for adding properties to result

bruns added a comment. IMHO this test is pointless, as the contents of the variant map is inaccessible, it is only an intermediate storage. The correct way to retrieve the data is via `Result::document()`. Or you can just accept D19087 , which already

D17302: Add test for adding properties to result

astippich added a comment. ping? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich added a comment. ping REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich updated this revision to Diff 52894. astippich added a comment. - mark expected fail REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17302?vs=48688=52894 BRANCH test_result REVISION DETAIL https://phabricator.kde.org/D17302 AFFECTED FILES

D17302: Add test for adding properties to result

astippich added a comment. In D17302#417646 , @bruns wrote: > In D17302#410231 , @astippich wrote: > > > In D17302#410189 , @bruns wrote: > > > > >

D17302: Add test for adding properties to result

bruns added a comment. In D17302#410231 , @astippich wrote: > In D17302#410189 , @bruns wrote: > > > Currently, both > > `Result::add(prop, "value1"); Result::add(prop, "value2");` > > and >

D17302: Add test for adding properties to result

astippich added a comment. In D17302#410189 , @bruns wrote: > Currently, both > `Result::add(prop, "value1"); Result::add(prop, "value2");` > and > `Result::add(prop, {"value1", "value2"});` > are serialized (JSON) in the same way as

D17302: Add test for adding properties to result

bruns added a comment. Currently, both `Result::add(prop, "value1"); Result::add(prop, "value2");` and `Result::add(prop, {"value1", "value2"});` are serialized (JSON) in the same way as `{prop: ["value1", "value2"]}` by Baloo, which is IMHO fine. On the other hand,

D17302: Add test for adding properties to result

astippich added a comment. Is this a please don't merge or can I land it? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich added inline comments. INLINE COMMENTS > bruns wrote in resulttest.cpp:63 > This looks wrong to me ... > How many items do you get when you append "keyword3" first and ["keyword1", > "keyword2"] next? It's the same. The properties will get merged regardless of their form if the

D17302: Add test for adding properties to result

bruns added inline comments. INLINE COMMENTS > resulttest.cpp:63 > +QCOMPARE(keywords->type(), QVariant::List); > +QCOMPARE(keywords->toList().count(), 2); > + This looks wrong to me ... How many items do you get when you append "keyword3" first and ["keyword1", "keyword2"]

D17302: Add test for adding properties to result

astippich added a comment. Since nobody seems to care and this is just testing the status quo, I'll merge it next week unless somebody objects REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, ashaposhnikov,

D17302: Add test for adding properties to result

astippich added a comment. ping! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17302: Add test for adding properties to result

astippich retitled this revision from "add simple test for string merging" to "Add test for adding properties to result". astippich edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D17302 To: astippich, #baloo, bruns Cc: