D25996: KRemoteEncoding::encoding: don't return temporary. Found with clazy.

[Christian Ehrlicher]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:d80cb8c96eef: KRemoteEncoding::encoding: dont 
return temporary. Found with clazy. (authored by chehrlic).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25996?vs=71529=71572

REVISION DETAIL
  https://phabricator.kde.org/D25996

AFFECTED FILES
  src/core/kremoteencoding.cpp

To: chehrlic, thiago, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25997: KFilePlacesModel: properly parent tagsLister to avoid memleak. Introduced with D7700

[Christian Ehrlicher]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:be9537998890: KFilePlacesModel: properly parent 
tagsLister to avoid memleak. Introduced with… (authored by chehrlic).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25997?vs=71530=71571

REVISION DETAIL
  https://phabricator.kde.org/D25997

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: chehrlic, dfaure, nicolasfella, ngraham, #dolphin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25995: HTTP ioslave: call correct base class in virtual_hook(). The base of HTTP ioslave is TCPSlaveBase, not SlaveBase.

[Christian Ehrlicher]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:6f1628706490: HTTP ioslave: call correct base class in 
virtual_hook(). The base of HTTP… (authored by chehrlic).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25995?vs=71528=71570

REVISION DETAIL
  https://phabricator.kde.org/D25995

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: chehrlic, dfaure, ngraham, #dolphin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25996: KRemoteEncoding::encoding: don't return temporary. Found with clazy.

[Christian Ehrlicher]

chehrlic added a comment.


  It was clazy, not me - just wanted to fix a memleak in kio :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D25996

To: chehrlic, thiago, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25997: KFilePlacesModel: properly parent tagsLister to avoid memleak. Introduced with D7700

[Christian Ehrlicher]

chehrlic added reviewers: dfaure, nicolasfella, ngraham, Dolphin.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D25997

To: chehrlic, dfaure, nicolasfella, ngraham, #dolphin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25996: KRemoteEncoding::encoding: don't return temporary. Found with clazy.

[Christian Ehrlicher]

chehrlic added reviewers: thiago, dfaure.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D25996

To: chehrlic, thiago, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25995: HTTP ioslave: call correct base class in virtual_hook(). The base of HTTP ioslave is TCPSlaveBase, not SlaveBase.

[Christian Ehrlicher]

chehrlic added reviewers: dfaure, ngraham, Dolphin.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D25995

To: chehrlic, dfaure, ngraham, #dolphin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25997: KFilePlacesModel: properly parent tagsLister to avoid memleak. Introduced with D7700

[Christian Ehrlicher]

chehrlic created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
chehrlic requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  kfileplacesmodel

REVISION DETAIL
  https://phabricator.kde.org/D25997

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: chehrlic
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25996: KRemoteEncoding::encoding: don't return temporary. Found with clazy.

[Christian Ehrlicher]

chehrlic created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
chehrlic requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  remoteencoding

REVISION DETAIL
  https://phabricator.kde.org/D25996

AFFECTED FILES
  src/core/kremoteencoding.cpp

To: chehrlic
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25995: HTTP ioslave: call correct base class in virtual_hook(). The base of HTTP ioslave is TCPSlaveBase, not SlaveBase.

[Christian Ehrlicher]

chehrlic created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
chehrlic requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  http

REVISION DETAIL
  https://phabricator.kde.org/D25995

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: chehrlic
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25994: KFilePlacesModel: properly parent tagsLister to avoid memleak. Introduced with D7700

[Christian Ehrlicher]

chehrlic abandoned this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D25994

To: chehrlic
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25994: KFilePlacesModel: properly parent tagsLister to avoid memleak. Introduced with D7700

[Christian Ehrlicher]

chehrlic created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
chehrlic requested review of this revision.

REVISION SUMMARY
  HTTP ioslave: call correct base class in virtual_hook(). The base of HTTP 
ioslave is TCPSlaveBase, not SlaveBase.
  
  KRemoteEncoding::encoding: don't return temporary. Found with clazy.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25994

AFFECTED FILES
  src/core/kremoteencoding.cpp
  src/filewidgets/kfileplacesmodel.cpp
  src/ioslaves/http/http.cpp

To: chehrlic
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D7700: Show list of tags in PlacesView

[Christian Ehrlicher]

chehrlic added inline comments.

INLINE COMMENTS

> kfileplacesmodel.cpp:172
> +  tags(),
> +  tagsLister(new KCoreDirLister())
>  {

While I way playing around with asan, I got a report that tagsLister is 
leaking. I can't see a deletion of this object anywhere, even in current code.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7700

To: nicolasfella, #dolphin, #kde_applications, alexeymin, ngraham, broulik
Cc: chehrlic, broulik, kde-frameworks-devel, bruns, rkflx, mmustac, spoorun, 
michaelh, renatoo, anthonyfieroni, cfeck, elvisangelaccio, emmanuelp, ngraham, 
alexeymin, #dolphin, pberestov, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, navarromorales, firef, 
andrebarros, mikesomov

D24841: Use modern way to set the C/CXX standad

[Christian Ehrlicher]

chehrlic added a comment.


  I would also use CMAKE_CXX_STANDARD_REQUIRED to make sure the compiler 
actually *can* c++11, otherwise the flag is only added when the compiler 
supports it. Should not make much difference nowadays but since it's in ECM...

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24841

To: vonreth, dfaure, cullmann
Cc: chehrlic, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D23789: RFC: Add ECMGenerateExportHeaders, for improved handling of deprecated API

[Christian Ehrlicher]

chehrlic added a comment.


  In D23789#538985 , @kossebau wrote:
  
  > >> - why has all Qt code not yet been adapted to 
QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those 
macros should not be used, but the version-less ones?
  > > 
  > > Because noone wanted to do the work and it was added late in the Qt5 
lifetime -> A lot of stuff was deprecated for a long time already (in Qt4 
times) and there is was a replacement since Qt5.0.0 so the macro was not needed 
(even though a lot of people got very angry about it). I added it for some new 
signals which created a lot of discussion since the old ones are widely used.
  >
  > Can you point to those discussions? Would be curious to learn what people's 
thought are.
  
  
  https://lists.qt-project.org/pipermail/development/2019-March/035343.html
  
  It's most likely a discussion when a deprecated / replaced function should 
really create a warning (esp. when the replacement was just recently added) and 
when the function should be removed completely - should a function deprecated 
in Qt5 really be removed in Qt6 or wait for Qt7 or don't remove it at all.
  
  > 
  > 
  >>> - why did you go for both QT_DEPRECATED_VERSION & 
QT_DEPRECATED_VERSION_X, are there places where no message will be wanted?
  >> 
  >> Just for consistency - we've QT_DEPRECATED and QT_DEPRECATED_X but I think 
QT_DEPRECATED should be deprecated by itself. I hope no reviewer will accept a 
QT_DEPRECATED anymore.
  > 
  > So you would also think that message-less variants will not be needed I 
understand. Okay. From what I tested with experimentally deploying the macros 
on some KF modules I almost always could add a useful message, so running short 
of reasons for keeping a message-less variant :)
  
  I think it should be enough to have one macro nowadays :)

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D23789

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D23789: RFC: Add ECMGenerateExportHeaders, for improved handling of deprecated API

[Christian Ehrlicher]

chehrlic added a comment.


  In D23789#536338 , @kossebau wrote:
  
  > Actual questions I have:
  >
  > - why is QT_DEPRECATED_WARNINGS_SINCE not officially documented? like, any 
plans to change that macro to something else?
  
  
  Just forgot it (and also the reviewers) I would guess. There is no plan to 
change it, at least none I'm aware of. I'm looking for an automatic generation 
of this macro. Since Qt6 switches to CMake I can maybe borrow some stuff from 
you ;)
  
  > - why has all Qt code not yet been adapted to 
QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those 
macros should not be used, but the version-less ones?
  
  Because noone wanted to do the work and it was added late in the Qt5 lifetime 
-> A lot of stuff was deprecated for a long time already (in Qt4 times) and 
there is was a replacement since Qt5.0.0 so the macro was not needed (even 
though a lot of people got very angry about it). I added it for some new 
signals which created a lot of discussion since the old ones are widely used.
  
  > - why did you go for both QT_DEPRECATED_VERSION & QT_DEPRECATED_VERSION_X, 
are there places where no message will be wanted?
  
  Just for consistency - we've QT_DEPRECATED and QT_DEPRECATED_X but I think 
QT_DEPRECATED should be deprecated by itself. I hope no reviewer will accept a 
QT_DEPRECATED anymore.
  
  The main problem with the macro is that you have to create a new one for each 
version so it will become lengthy. But I could not find a better solution 
somewhere - in the end it's only macro magic from the 80s... :(
  But for a library such a macro should be mandatory - your blog post explains 
the problem very good.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D23789

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D11346: Only set iconText() if actually changed

[Christian Ehrlicher]

chehrlic accepted this revision.
chehrlic added a comment.
This revision is now accepted and ready to land.


  This should at least fix the most cases as described in the bug report :)

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D11346

To: cfeck, #frameworks, chehrlic
Cc: chehrlic, michaelh, ngraham

D7926: Remove pf.path() from container before the reference got screwed up by it.remove()

[Christian Ehrlicher]

chehrlic closed this revision.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D7926

To: chehrlic, broulik
Cc: #frameworks

D7926: Remove pf.path() from container before the reference got screwed up by it.remove()

[Christian Ehrlicher]

chehrlic created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  With 
https://phabricator.kde.org/R293:a75db7aedfcd356b027774e0dc156c334d4cb869 I 
introduced a crash when removing a watched
  directory. The Problem is that it.remove() invalidates the reference to 
PendingFile which
  leads to a crash... :(
  I found no bugreport about this on bugs.kde.org until now.

TEST PLAN
  Remove a directory, baloo should never crash when removing a watched directory

REPOSITORY
  R293 Baloo

BRANCH
  fix_crash_pendingfilequeue

REVISION DETAIL
  https://phabricator.kde.org/D7926

AFFECTED FILES
  src/file/pendingfilequeue.cpp

To: chehrlic
Cc: #frameworks

Re: Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/
---

(Updated Aug. 24, 2017, 5:48 a.m.)


Review request for Baloo.


Changes
---

Avoid one more string allocation in FileIndexerConfig::shouldFolderBeIndexed() 
by using midRef() early, rebase to current HEAD


Repository: baloo


Description
---

Use the new QStringRef class to avoid creation of temporary QString objects.
Since every filename is sent through FileIndexerConfig::shouldFileBeIndexed(), 
I think this is a good optimization although now there are two functions for 
FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
with QString and one with QStringRef ...


Diffs (updated)
-

  src/engine/queryparser.cpp 9c3a4b60 
  src/file/fileindexerconfig.h e2466bdd 
  src/file/fileindexerconfig.cpp 0e4f4c45 
  src/file/modifiedfileindexer.cpp d1f5011b 
  src/file/newfileindexer.cpp 7e196229 
  src/file/regexpcache.h 60d370de 
  src/file/regexpcache.cpp 61f07d61 
  src/file/xattrindexer.cpp 3daecef6 
  src/kioslaves/timeline/timelinetools.cpp 26e44ee8 

Diff: https://git.reviewboard.kde.org/r/129349/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129378: [baloo] Avoid some useless copies by using const references instead

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129378/
---

(Updated Aug. 24, 2017, 7:06 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo.


Changes
---

Submitted with commit a75db7aedfcd356b027774e0dc156c334d4cb869 by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

No functional changes, just adding some const & to avoid copies where possible.


Diffs
-

  src/engine/transaction.cpp a1e4fe3 
  src/engine/writetransaction.cpp 171f5ba 
  src/file/fileindexscheduler.cpp d50f2be 
  src/file/pendingfilequeue.cpp 685bf9e 
  tests/engine/basicindexingtest.cpp 1db4d15 

Diff: https://git.reviewboard.kde.org/r/129378/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129378: [baloo] Avoid some useless copies by using const references instead

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129378/#review103640
---



Anyone interested? Otherwise I'll discard this review in the next days

- Christian Ehrlicher


On Nov. 11, 2016, 4:58 nachm., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129378/
> ---
> 
> (Updated Nov. 11, 2016, 4:58 nachm.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> No functional changes, just adding some const & to avoid copies where 
> possible.
> 
> 
> Diffs
> -
> 
>   src/engine/transaction.cpp a1e4fe3 
>   src/engine/writetransaction.cpp 171f5ba 
>   src/file/fileindexscheduler.cpp d50f2be 
>   src/file/pendingfilequeue.cpp 685bf9e 
>   tests/engine/basicindexingtest.cpp 1db4d15 
> 
> Diff: https://git.reviewboard.kde.org/r/129378/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Re: Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/#review103639
---



Anyone interested? Otherwise I'll discard this review in the next days

- Christian Ehrlicher


On Dez. 13, 2016, 7:39 nachm., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129349/
> ---
> 
> (Updated Dez. 13, 2016, 7:39 nachm.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Use the new QStringRef class to avoid creation of temporary QString objects.
> Since every filename is sent through 
> FileIndexerConfig::shouldFileBeIndexed(), I think this is a good optimization 
> although now there are two functions for 
> FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
> with QString and one with QStringRef ...
> 
> 
> Diffs
> -
> 
>   src/engine/queryparser.cpp d096bf3 
>   src/file/fileindexerconfig.h 10eacd5 
>   src/file/fileindexerconfig.cpp 0e4f4c4 
>   src/file/modifiedfileindexer.cpp d1f5011 
>   src/file/newfileindexer.cpp 7e19622 
>   src/file/regexpcache.h 60d370d 
>   src/file/regexpcache.cpp 61f07d6 
>   src/file/xattrindexer.cpp 3daecef 
>   src/kioslaves/tags/kio_tags.cpp de2e6d7 
>   src/kioslaves/timeline/timelinetools.cpp 26e44ee 
> 
> Diff: https://git.reviewboard.kde.org/r/129349/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Re: Review Request 129650: [baloo] Remove useless includes

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129650/
---

(Updated Aug. 23, 2017, 7:20 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo.


Changes
---

Submitted with commit 1ba4c1375ba74d6eb61f70d7215ed160735d269e by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

In mainadaptor there are a lot of useless forward declarations and includes - 
don't know why and therefore a review request here :)
It's no public API so this shouldn't be an argument.


Diffs
-

  src/file/mainadaptor.h b5c8bd3 
  src/file/mainadaptor.cpp 933b1d0 

Diff: https://git.reviewboard.kde.org/r/129650/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: KF5-based kdesvn doesn't display the authorization dialog

[Christian Ehrlicher]

Am 14.02.2017 um 10:43 schrieb Kevin Funk:

CC'ed maintainer. I'm not sure he's reading kde-devel@.

On Sunday, 29 January 2017 09:24:26 CET Safa Alfulaij wrote:

Hello world.

I have updated my *kdesvn* from KDE4 to KF5 recently, but didn't tried to
use it back then.
I'm having a problem that is that kdesvn doesn't ask me for my ssh
password, and just displays the common error in the errors box:


Hello,

there's already a bug about this: 
https://bugs.kde.org/show_bug.cgi?id=375776
I've currently no time to look into this issue but hope to get back to 
kdesvn in march.



Thx,
Christian

Re: Dropping the svn kio from kdesdk-kioslaves

[Christian Ehrlicher]

Am 16.12.2016 um 11:32 schrieb Kevin Funk:

If the SVN kioslave is standalone, wouldn't it make more sense to actually
keep it in a separate repository/package? So users who just want the Dolphin
integration can just use that?

Wouldn't it make more sense to move your kioslave implementation into kdesdk-
kioslaves, replacing the older copy?

I'm not sure whether Albert just wants to kill the repository or he's just
phasing old unmaintained software. In case of the latter, updating the copy
with the better maintained version probably makes more sense.
I took a look into kdesdk-kioslaves - the code for svn there is 
completely different from kdesvn kioslave. Therefore I would suggest to 
remove kdesdk-kioslaves/svn because I won't maintain two different 
implementations for one protocol.
A distro packager can separate the kdesvn kioslave into a separate 
package, at least this is what I would expect looking at the kdesvn code.



Christian

Re: Dropping the svn kio from kdesdk-kioslaves

[Christian Ehrlicher]

Am 15.12.2016 um 23:12 schrieb Kevin Funk:

On Thursday, 15 December 2016 22:14:38 CET Albert Astals Cid wrote:

Why?
  * It doesn't compile with newer libsvn
  * It's kdelibs4-based

Heya,

I know a few people that use kdesvn, how's the relation to that?

I saw that Christian (CC'ed) put quite some effort in porting kdesvn over to a
KF5-based kdelibs4support-free code base. If I understand correctly kdesvn.git
even contains a copy of the KIO SVN implementation (c.f. kdesvn.git:src/
kiosvn)?

Please elaborate :)
You're correct - kdesvn contains a kioslave for svn and I ported it to 
KF5 (just released kdesvn 2.0.0 some days ago). I don't know if 
kdesdk-kioslaves has more functionality but from my point of view it can 
be dropped.


Cheers,
Christian

Re: Dropping the svn kio from kdesdk-kioslaves

[Christian Ehrlicher]

Am 15.12.2016 um 23:12 schrieb Kevin Funk:

On Thursday, 15 December 2016 22:14:38 CET Albert Astals Cid wrote:

Why?
  * It doesn't compile with newer libsvn
  * It's kdelibs4-based

Heya,

I know a few people that use kdesvn, how's the relation to that?

I saw that Christian (CC'ed) put quite some effort in porting kdesvn over to a
KF5-based kdelibs4support-free code base. If I understand correctly kdesvn.git
even contains a copy of the KIO SVN implementation (c.f. kdesvn.git:src/
kiosvn)?

Please elaborate :)
You're correct - kdesvn contains a kioslave for svn and I ported it to 
KF5 (just released kdesvn 2.0.0 some days ago). I don't know if 
kdesdk-kioslaves has more functionality but from my point of view it can 
be dropped.


Cheers,
Christian

Review Request 129650: [baloo] Remove useless includes

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129650/
---

Review request for Baloo.


Repository: baloo


Description
---

In mainadaptor there are a lot of useless forward declarations and includes - 
don't know why and therefore a review request here :)
It's no public API so this shouldn't be an argument.


Diffs
-

  src/file/mainadaptor.h b5c8bd3 
  src/file/mainadaptor.cpp 933b1d0 

Diff: https://git.reviewboard.kde.org/r/129650/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/
---

(Updated Dec. 13, 2016, 7:39 p.m.)


Review request for Baloo.


Changes
---

Changed the last issue to use QVector and QRegularExpression 
instead QRegExp (tested it with local testcase).


Repository: baloo


Description
---

Use the new QStringRef class to avoid creation of temporary QString objects.
Since every filename is sent through FileIndexerConfig::shouldFileBeIndexed(), 
I think this is a good optimization although now there are two functions for 
FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
with QString and one with QStringRef ...


Diffs (updated)
-

  src/engine/queryparser.cpp d096bf3 
  src/file/fileindexerconfig.h 10eacd5 
  src/file/fileindexerconfig.cpp 0e4f4c4 
  src/file/modifiedfileindexer.cpp d1f5011 
  src/file/newfileindexer.cpp 7e19622 
  src/file/regexpcache.h 60d370d 
  src/file/regexpcache.cpp 61f07d6 
  src/file/xattrindexer.cpp 3daecef 
  src/kioslaves/tags/kio_tags.cpp de2e6d7 
  src/kioslaves/timeline/timelinetools.cpp 26e44ee 

Diff: https://git.reviewboard.kde.org/r/129349/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

> On Dez. 13, 2016, 3:08 vorm., Aleix Pol Gonzalez wrote:
> > src/kioslaves/timeline/timelinetools.cpp, line 98
> > <https://git.reviewboard.kde.org/r/129349/diff/2/?file=487336#file487336line98>
> >
> > Use `split(QLatin1Char('/'),...)`
> > Also this can be changed into splitRef?

I thought about this too but since s_dateRegexp is a QRegExp (and not a 
QRegularExpression) I thought that the outcome is not worth the change. Maybe I 
should change s_dateRegexp to a QRegularExpression ...


- Christian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/#review101405
---


On Dez. 12, 2016, 6:48 nachm., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129349/
> ---
> 
> (Updated Dez. 12, 2016, 6:48 nachm.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Use the new QStringRef class to avoid creation of temporary QString objects.
> Since every filename is sent through 
> FileIndexerConfig::shouldFileBeIndexed(), I think this is a good optimization 
> although now there are two functions for 
> FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
> with QString and one with QStringRef ...
> 
> 
> Diffs
> -
> 
>   src/engine/queryparser.cpp d096bf3 
>   src/file/fileindexerconfig.h 10eacd5 
>   src/file/fileindexerconfig.cpp 0e4f4c4 
>   src/file/modifiedfileindexer.cpp d1f5011 
>   src/file/newfileindexer.cpp 7e19622 
>   src/file/regexpcache.h 60d370d 
>   src/file/regexpcache.cpp 61f07d6 
>   src/file/xattrindexer.cpp 3daecef 
>   src/kioslaves/tags/kio_tags.cpp de2e6d7 
>   src/kioslaves/timeline/timelinetools.cpp 26e44ee 
> 
> Diff: https://git.reviewboard.kde.org/r/129349/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Re: Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/
---

(Updated Dec. 12, 2016, 6:48 p.m.)


Review request for Baloo.


Changes
---

Updated diff to fix issues


Repository: baloo


Description
---

Use the new QStringRef class to avoid creation of temporary QString objects.
Since every filename is sent through FileIndexerConfig::shouldFileBeIndexed(), 
I think this is a good optimization although now there are two functions for 
FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
with QString and one with QStringRef ...


Diffs (updated)
-

  src/engine/queryparser.cpp d096bf3 
  src/file/fileindexerconfig.h 10eacd5 
  src/file/fileindexerconfig.cpp 0e4f4c4 
  src/file/modifiedfileindexer.cpp d1f5011 
  src/file/newfileindexer.cpp 7e19622 
  src/file/regexpcache.h 60d370d 
  src/file/regexpcache.cpp 61f07d6 
  src/file/xattrindexer.cpp 3daecef 
  src/kioslaves/tags/kio_tags.cpp de2e6d7 
  src/kioslaves/timeline/timelinetools.cpp 26e44ee 

Diff: https://git.reviewboard.kde.org/r/129349/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/#review101362
---



Ping, no interest anyone?

- Christian Ehrlicher


On Nov. 6, 2016, 1:45 nachm., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129349/
> ---
> 
> (Updated Nov. 6, 2016, 1:45 nachm.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Use the new QStringRef class to avoid creation of temporary QString objects.
> Since every filename is sent through 
> FileIndexerConfig::shouldFileBeIndexed(), I think this is a good optimization 
> although now there are two functions for 
> FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
> with QString and one with QStringRef ...
> 
> 
> Diffs
> -
> 
>   src/engine/queryparser.cpp d096bf3 
>   src/file/fileindexerconfig.h 10eacd5 
>   src/file/fileindexerconfig.cpp 0e4f4c4 
>   src/file/modifiedfileindexer.cpp d1f5011 
>   src/file/newfileindexer.cpp 7e19622 
>   src/file/regexpcache.h 60d370d 
>   src/file/regexpcache.cpp 61f07d6 
>   src/file/xattrindexer.cpp 3daecef 
>   src/kioslaves/tags/kio_tags.cpp de2e6d7 
> 
> Diff: https://git.reviewboard.kde.org/r/129349/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Re: Review Request 129378: [baloo] Avoid some useless copies by using const references instead

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129378/#review101361
---



Ping, no interest anyone?

- Christian Ehrlicher


On Nov. 11, 2016, 4:58 nachm., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129378/
> ---
> 
> (Updated Nov. 11, 2016, 4:58 nachm.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> No functional changes, just adding some const & to avoid copies where 
> possible.
> 
> 
> Diffs
> -
> 
>   src/engine/transaction.cpp a1e4fe3 
>   src/engine/writetransaction.cpp 171f5ba 
>   src/file/fileindexscheduler.cpp d50f2be 
>   src/file/pendingfilequeue.cpp 685bf9e 
>   tests/engine/basicindexingtest.cpp 1db4d15 
> 
> Diff: https://git.reviewboard.kde.org/r/129378/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Re: Review Request 129379: [baloo] Remove unused functions

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129379/
---

(Updated Nov. 15, 2016, 7:58 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo and David Faure.


Changes
---

Submitted with commit e40689fb56fbcd12ca4eeaef1b44e93f5afc6d7d by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

As discussed in REVIEW 129186 there is a lot of bit rotting in coding.cpp/.h. 
With this patch I removed all the unused stuff and moved out a temporary buffer 
to avoid reallocating this buffer on every call to putDifferentialVarInt32()


Diffs
-

  src/codecs/coding.h 3d55a40 
  src/codecs/coding.cpp 73e2fa2 
  src/codecs/positioncodec.cpp 22f4895 

Diff: https://git.reviewboard.kde.org/r/129379/diff/


Testing
---

Unittests still working as expected


Thanks,

Christian Ehrlicher

Re: Review Request 129378: [baloo] Avoid some useless copies by using const references instead

[Christian Ehrlicher]

> On Nov. 11, 2016, 11:42 nachm., Aleix Pol Gonzalez wrote:
> > I don't think this is saving anything, the move constructor should be 
> > taking care of it.
> > http://en.cppreference.com/w/cpp/language/move_constructor

I'm sorry but where do you see move ctors here in the first place? it.next() is 
returning a simple const ref which must not be modified. How should this data 
moved from this const iterator to the local object?


- Christian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129378/#review100797
---


On Nov. 11, 2016, 4:58 nachm., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129378/
> ---
> 
> (Updated Nov. 11, 2016, 4:58 nachm.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> No functional changes, just adding some const & to avoid copies where 
> possible.
> 
> 
> Diffs
> -
> 
>   src/engine/transaction.cpp a1e4fe3 
>   src/engine/writetransaction.cpp 171f5ba 
>   src/file/fileindexscheduler.cpp d50f2be 
>   src/file/pendingfilequeue.cpp 685bf9e 
>   tests/engine/basicindexingtest.cpp 1db4d15 
> 
> Diff: https://git.reviewboard.kde.org/r/129378/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Review Request 129379: [baloo] Remove unused functions

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129379/
---

Review request for Baloo and David Faure.


Repository: baloo


Description
---

As discussed in REVIEW 129186 there is a lot of bit rotting in coding.cpp/.h. 
With this patch I removed all the unused stuff and moved out a temporary buffer 
to avoid reallocating this buffer on every call to putDifferentialVarInt32()


Diffs
-

  src/codecs/coding.h 3d55a40 
  src/codecs/coding.cpp 73e2fa2 
  src/codecs/positioncodec.cpp 22f4895 

Diff: https://git.reviewboard.kde.org/r/129379/diff/


Testing
---

Unittests still working as expected


Thanks,

Christian Ehrlicher

Review Request 129378: [baloo] Avoid some useless copies by using const references instead

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129378/
---

Review request for Baloo.


Repository: baloo


Description
---

No functional changes, just adding some const & to avoid copies where possible.


Diffs
-

  src/engine/transaction.cpp a1e4fe3 
  src/engine/writetransaction.cpp 171f5ba 
  src/file/fileindexscheduler.cpp d50f2be 
  src/file/pendingfilequeue.cpp 685bf9e 
  tests/engine/basicindexingtest.cpp 1db4d15 

Diff: https://git.reviewboard.kde.org/r/129378/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Nov. 7, 2016, 6:44 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, KDE Frameworks and Vishesh Handa.


Changes
---

Submitted with commit 2717080c46072b2a2998cb7d043a7cbcefa6a843 by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Ping! No interest in this patch? Should I discard it?


Diffs
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Nov. 7, 2016, 6:44 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, KDE Frameworks and Vishesh Handa.


Changes
---

Submitted with commit 2717080c46072b2a2998cb7d043a7cbcefa6a843 by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Ping! No interest in this patch? Should I discard it?


Diffs
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Review Request 129349: [baloo] add (and use) FileIndexerConfig::shouldFileBeIndexed(const QStringRef&)

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129349/
---

Review request for Baloo.


Repository: baloo


Description
---

Use the new QStringRef class to avoid creation of temporary QString objects.
Since every filename is sent through FileIndexerConfig::shouldFileBeIndexed(), 
I think this is a good optimization although now there are two functions for 
FileIndexerConfig::shouldFileBeIndexed() and RegExpCache::exactMatch() - one 
with QString and one with QStringRef ...


Diffs
-

  src/engine/queryparser.cpp d096bf3 
  src/file/fileindexerconfig.h 10eacd5 
  src/file/fileindexerconfig.cpp 0e4f4c4 
  src/file/modifiedfileindexer.cpp d1f5011 
  src/file/newfileindexer.cpp 7e19622 
  src/file/regexpcache.h 60d370d 
  src/file/regexpcache.cpp 61f07d6 
  src/file/xattrindexer.cpp 3daecef 
  src/kioslaves/tags/kio_tags.cpp de2e6d7 

Diff: https://git.reviewboard.kde.org/r/129349/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Nov. 6, 2016, 12:58 p.m.)


Review request for Baloo, KDE Frameworks and Vishesh Handa.


Changes
---

New patch with changes from comments


Repository: baloo


Description
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Ping! No interest in this patch? Should I discard it?


Diffs (updated)
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Nov. 6, 2016, 12:58 p.m.)


Review request for Baloo, KDE Frameworks and Vishesh Handa.


Changes
---

New patch with changes from comments


Repository: baloo


Description
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Ping! No interest in this patch? Should I discard it?


Diffs (updated)
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Re: Review Request 129183: [baloo] CLazy fixes: removed unused member var, add Q_OBJECT

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129183/
---

(Updated Nov. 6, 2016, 10:56 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo.


Changes
---

Submitted with commit edcda57297a1fc4ff74555afd402cf9c69a2db25 by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

CLayz warns about an unused member var in IOHandler and a missing Q_OBJECT 
macro in TimeEstimator which produces a lot of noise in the compile output.

Ping! No interest in this patch? Should I discard it?


Diffs
-

  src/file/extractor/app.cpp 6843165 
  src/file/extractor/autotests/iohandlertest.cpp 2c8eeaa 
  src/file/extractor/iohandler.h c0dfbbb 
  src/file/extractor/iohandler.cpp b49a2b2 
  src/file/timeestimator.h 11137d3 

Diff: https://git.reviewboard.kde.org/r/129183/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Nov. 5, 2016, 11:39 p.m.)


Review request for Baloo, KDE Frameworks and Vishesh Handa.


Repository: baloo


Description
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Ping! No interest in this patch? Should I discard it?


Diffs
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Re: Review Request 129183: [baloo] CLazy fixes: removed unused member var, add Q_OBJECT

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129183/
---

(Updated Nov. 5, 2016, 11:37 vorm.)


Review request for Baloo.


Repository: baloo


Description (updated)
---

CLayz warns about an unused member var in IOHandler and a missing Q_OBJECT 
macro in TimeEstimator which produces a lot of noise in the compile output.

Ping! No interest in this patch? Should I discard it?


Diffs
-

  src/file/extractor/app.cpp 6843165 
  src/file/extractor/autotests/iohandlertest.cpp 2c8eeaa 
  src/file/extractor/iohandler.h c0dfbbb 
  src/file/extractor/iohandler.cpp b49a2b2 
  src/file/timeestimator.h 11137d3 

Diff: https://git.reviewboard.kde.org/r/129183/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Nov. 5, 2016, 11:37 vorm.)


Review request for Baloo and Vishesh Handa.


Repository: baloo


Description (updated)
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Ping! No interest in this patch? Should I discard it?


Diffs
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Re: Review Request 129185: [baloo] Add a small benchmark for PositionCodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129185/
---

(Updated Nov. 5, 2016, 11:33 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo and Vishesh Handa.


Changes
---

Submitted with commit 8ca69dcb32a38027f4ec319ae112ca382b145733 by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

This small test is used to test if the changes inside PostingCodec::encode() do 
not break anything. It also does some benchmarking to show the speedup.

Ping?


Diffs
-

  autotests/benchmarks/positioncodecbenchmark.cpp e7aeebf 
  autotests/unit/codecs/CMakeLists.txt f65d633 
  autotests/unit/codecs/positioncodectest.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/129185/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129185: [baloo] Add a small benchmark for PositionCodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129185/
---

(Updated Okt. 23, 2016, 6:21 nachm.)


Review request for Baloo.


Changes
---

Any chance to get these changes to baloo reviewed?


Repository: baloo


Description (updated)
---

This small test is used to test if the changes inside PostingCodec::encode() do 
not break anything. It also does some benchmarking to show the speedup.

Ping?


Diffs
-

  autotests/benchmarks/positioncodecbenchmark.cpp e7aeebf 
  autotests/unit/codecs/CMakeLists.txt f65d633 
  autotests/unit/codecs/positioncodectest.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/129185/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129185: [baloo] Add a small benchmark for PositionCodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129185/
---

(Updated Oct. 17, 2016, 5:59 p.m.)


Review request for Baloo.


Changes
---

Found out that there was already a small benchmark but imo the usecase (very 
few positions in a file) is no real-world example - at least during my 
debugging I there were at least 1k positions. Therefore I added my data too. I 
also added a small unit test to check the encoded/decoded output in 
autotests/unit/codec


Repository: baloo


Description
---

This small test is used to test if the changes inside PostingCodec::encode() do 
not break anything. It also does some benchmarking to show the speedup.


Diffs (updated)
-

  autotests/benchmarks/positioncodecbenchmark.cpp e7aeebf 
  autotests/unit/codecs/CMakeLists.txt f65d633 
  autotests/unit/codecs/positioncodectest.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/129185/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129186/
---

(Updated Okt. 17, 2016, 5:30 vorm.)


Review request for Baloo.


Changes
---

Postingcode -> Positioncodec :)


Summary (updated)
-

[baloo] Speedup Positioncodec::encode() 


Repository: baloo


Description
---

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding 
every single encoded int32 to the resulting bytearray which results in a lot of 
small memcpy operations.
The idea is to use a preallocated QByteArray and directly encode the integers 
into this buffer. This makes the code a little bit more complex but the speedup 
should be gain enough for this.


Diffs
-

  src/codecs/coding.cpp 5961077 

Diff: https://git.reviewboard.kde.org/r/129186/diff/


Testing
---

positoncodectest calculates the same md5sum as before.


Thanks,

Christian Ehrlicher

Re: Review Request 129185: [baloo] Add a small benchmark for PostingCodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129185/
---

(Updated Oct. 15, 2016, 11:46 a.m.)


Review request for Baloo.


Changes
---

Use QTest to make it a real unit test including a benchmark


Repository: baloo


Description
---

This small test is used to test if the changes inside PostingCodec::encode() do 
not break anything. It also does some benchmarking to show the speedup.


Diffs (updated)
-

  tests/benchmarks/CMakeLists.txt 0d74718 
  tests/benchmarks/positioncodectest.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/129185/diff/


Testing
---


Thanks,

Christian Ehrlicher

Re: Review Request 129182: [baloo] CLazy fixes: use const ref in for loop instead object

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129182/
---

(Updated Oct. 15, 2016, 11:16 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo.


Changes
---

Submitted with commit e6bd357153a790ef66dc803cd3451270e9311134 by Christian 
Ehrlicher to branch master.


Repository: baloo


Description
---

While trying to figure out why indexing my files is so slow, I compiled baloo 
on my system with clazy and found some unnecessary copies in foreach loops. 
It's not related to my problem and not really a performance critical part but 
imo should be fixed anyway


Diffs
-

  src/engine/transaction.cpp 908a81f 
  src/file/fileindexerconfig.cpp ccb5d80 
  src/file/pendingfilequeue.cpp 489f100 
  tests/file/storagedevicestest.cpp f269c81 

Diff: https://git.reviewboard.kde.org/r/129182/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Re: Review Request 129183: [baloo] CLazy fixes: removed unused member var, add Q_OBJECT

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129183/
---

(Updated Oct. 14, 2016, 9:13 p.m.)


Review request for Baloo.


Changes
---

Modify IOHandler class to not take the second (unused) argument at all instead 
ignoring it


Repository: baloo


Description
---

CLayz warns about an unused member var in IOHandler and a missing Q_OBJECT 
macro in TimeEstimator which produces a lot of noise in the compile output.


Diffs (updated)
-

  src/file/extractor/app.cpp 6843165 
  src/file/extractor/autotests/iohandlertest.cpp 2c8eeaa 
  src/file/extractor/iohandler.h c0dfbbb 
  src/file/extractor/iohandler.cpp b49a2b2 
  src/file/timeestimator.h 11137d3 

Diff: https://git.reviewboard.kde.org/r/129183/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Re: Review Request 129182: CLazy fixes: use const ref in for loop instead object

[Christian Ehrlicher]

> On Oct. 14, 2016, 8:58 p.m., Aleix Pol Gonzalez wrote:
> > src/engine/transaction.cpp, line 457
> > <https://git.reviewboard.kde.org/r/129182/diff/1/?file=482195#file482195line457>
> >
> > add const

The question here is - why create a temporary QList here at all - I'm now using 
Q_FOREACH on the map directly


- Christian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129182/#review17
---


On Oct. 14, 2016, 9:11 p.m., Christian Ehrlicher wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129182/
> ---
> 
> (Updated Oct. 14, 2016, 9:11 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> While trying to figure out why indexing my files is so slow, I compiled baloo 
> on my system with clazy and found some unnecessary copies in foreach loops. 
> It's not related to my problem and not really a performance critical part but 
> imo should be fixed anyway
> 
> 
> Diffs
> -
> 
>   src/engine/transaction.cpp 908a81f 
>   src/file/fileindexerconfig.cpp ccb5d80 
>   src/file/pendingfilequeue.cpp 489f100 
>   tests/file/storagedevicestest.cpp f269c81 
> 
> Diff: https://git.reviewboard.kde.org/r/129182/diff/
> 
> 
> Testing
> ---
> 
> Compiles fine
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

Re: Review Request 129182: CLazy fixes: use const ref in for loop instead object

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129182/
---

(Updated Oct. 14, 2016, 9:11 p.m.)


Review request for Baloo.


Changes
---

Use Q_FOREACH() on map directly instead create a temporary QList object with 
map.values()


Repository: baloo


Description
---

While trying to figure out why indexing my files is so slow, I compiled baloo 
on my system with clazy and found some unnecessary copies in foreach loops. 
It's not related to my problem and not really a performance critical part but 
imo should be fixed anyway


Diffs (updated)
-

  src/engine/transaction.cpp 908a81f 
  src/file/fileindexerconfig.cpp ccb5d80 
  src/file/pendingfilequeue.cpp 489f100 
  tests/file/storagedevicestest.cpp f269c81 

Diff: https://git.reviewboard.kde.org/r/129182/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Re: Review Request 129183: [baloo] CLazy fixes: removed unused member var, add Q_OBJECT

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129183/
---

(Updated Oct. 14, 2016, 8:58 p.m.)


Review request for Baloo.


Summary (updated)
-

[baloo] CLazy fixes: removed unused member var, add Q_OBJECT


Repository: baloo


Description
---

CLayz warns about an unused member var in IOHandler and a missing Q_OBJECT 
macro in TimeEstimator which produces a lot of noise in the compile output.


Diffs
-

  src/file/extractor/iohandler.h c0dfbbb 
  src/file/extractor/iohandler.cpp b49a2b2 
  src/file/timeestimator.h 11137d3 

Diff: https://git.reviewboard.kde.org/r/129183/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Review Request 129185: [baloo] Add a small benchmark for PostingCodec::encode()

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129185/
---

Review request for Baloo.


Repository: baloo


Description
---

This small test is used to test if the changes inside PostingCodec::encode() do 
not break anything. It also does some benchmarking to show the speedup.


Diffs
-

  tests/benchmarks/CMakeLists.txt 0d74718 
  tests/benchmarks/positioncodectest.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/129185/diff/


Testing
---


Thanks,

Christian Ehrlicher

Review Request 129183: CLazy fixes: removed unused member var, add Q_OBJECT

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129183/
---

Review request for Baloo.


Repository: baloo


Description
---

CLayz warns about an unused member var in IOHandler and a missing Q_OBJECT 
macro in TimeEstimator which produces a lot of noise in the compile output.


Diffs
-

  src/file/extractor/iohandler.h c0dfbbb 
  src/file/extractor/iohandler.cpp b49a2b2 
  src/file/timeestimator.h 11137d3 

Diff: https://git.reviewboard.kde.org/r/129183/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Review Request 129182: CLazy fixes: use const ref in for loop instead object

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129182/
---

Review request for Baloo.


Repository: baloo


Description
---

While trying to figure out why indexing my files is so slow, I compiled baloo 
on my system with clazy and found some unnecessary copies in foreach loops. 
It's not related to my problem and not really a performance critical part but 
imo should be fixed anyway


Diffs
-

  src/engine/transaction.cpp 908a81f 
  src/file/fileindexerconfig.cpp ccb5d80 
  src/file/pendingfilequeue.cpp 489f100 
  tests/file/storagedevicestest.cpp f269c81 

Diff: https://git.reviewboard.kde.org/r/129182/diff/


Testing
---

Compiles fine


Thanks,

Christian Ehrlicher

Re: Review Request 126392: Fix some Clazy warnings in kcoreaddons

[Christian Ehrlicher]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126392/#review90049
---



src/lib/io/kbackup.cpp (line 114)
<https://git.reviewboard.kde.org/r/126392/#comment61706>

Shouldn't this be toLocal8Bit()?



src/lib/io/kdirwatch_p.h (line 107)
<https://git.reviewboard.kde.org/r/126392/#comment61705>

const



src/lib/io/kurlmimedata.cpp (line 59)
<https://git.reviewboard.kde.org/r/126392/#comment61704>

const? Or would this be an ABI breakage?



src/lib/text/kmacroexpander.cpp (line 74)
<https://git.reviewboard.kde.org/r/126392/#comment61703>

Qt5 has a new overload QStringList::join(QChar) -> join(QLatin1Char(' '))


- Christian Ehrlicher


On Dez. 18, 2015, 7:32 nachm., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126392/
> ---
> 
> (Updated Dez. 18, 2015, 7:32 nachm.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Fix some Clazy warnings:
> - unneeded heap allocation with QString
> - midRef() non-usage
> - isEmpty() non-usage
> 
> 
> Diffs
> -
> 
>   autotests/ktexttohtmltest.cpp 81c3fc0 
>   src/lib/io/kbackup.h ead2de5 
>   src/lib/io/kbackup.cpp ec8b8b2 
>   src/lib/io/kdirwatch.cpp 34b32b7 
>   src/lib/io/kdirwatch_p.h 4483961 
>   src/lib/io/kmessage.cpp 305318c 
>   src/lib/io/kprocess.cpp 1df61c0 
>   src/lib/io/kurlmimedata.cpp 4095ee1 
>   src/lib/kaboutdata.h 0dbd7a0 
>   src/lib/kaboutdata.cpp 5d9a55e 
>   src/lib/randomness/krandom.cpp 2eb80d2 
>   src/lib/text/kmacroexpander.cpp 0d05ffd 
>   src/lib/text/kstringhandler.cpp 826ddcb 
>   src/lib/text/ktexttohtml.cpp d58f5c0 
>   src/lib/util/kshell.cpp 11892ce 
> 
> Diff: https://git.reviewboard.kde.org/r/126392/diff/
> 
> 
> Testing
> ---
> 
> Compiling without errors
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Why is C90 enforced in KDE?

[Christian Ehrlicher]

Am 20.12.2015 um 10:24 schrieb Thomas Lübking:

On Sonntag, 20. Dezember 2015 04:21:29 CEST, Kevin Kofler wrote:

The kdewin team should just point people to a flex.exe that produces 
files that work with the bitrotten C compiler included with Visual C++.


So we only need somebody who forks the hardly maintained flex/yacc 
tools for windows...
I think this one is actively maintained: 
https://sourceforge.net/projects/winflexbison/ and I used it to compile 
Qt5.5 with msvc2013 (somewhere in the qt docs there's a link to this 
project). So it *should* work with KF5 too. But someone has to test it. 
It's out of my scope because currently I don't have a working 
environment for KF5.



Christian

kdesvn moved to kdereview

[Christian Ehrlicher]

Hi,

after quite some time with no activity in kdesvn, we finally managed to 
get kdesvn in a shape to move it to extragear.

The next steps would be:
- create a 1.7.0 release (some help on what's needed to be done for this 
would be nice)

- fix bugs
- port to Qt5/KF5
- cleanup svnqt (esp. non thread-safe smart/shared_pointer, support for 
subversion <= 1.6, ...)


If I missed something which is needed for the move to extragear, please 
let me know.


Thx,
Christian

Re: State of kdesvn?

[Christian Ehrlicher]

Am 21.01.2015 um 23:14 schrieb Albert Astals Cid:

El Dimarts, 20 de gener de 2015, a les 20:14:39, Christian Ehrlicher va
escriure:

Hello,

I recently tried to fix some bugs in kdesvn but got stuck because there
is nobody who can review my patches. Rajko Albrecht as the original
maintainer stopped the development of kdesvn sometime in 2013 and the
other two project members (David Faure and Christophe Giboudeaux) have
no time to dig deep into the subversion api for a proper review.
Therefore I'm asking for help and suggestions how to go further. Maybe
there are some people on this list who are still working with subversion
and kdesvn and are interested in the further development of kdesvn.

It'd be easier if you had written some of the urls for reviewboard here, so we
could have clicked on them.
I'm looking more for a long-term solution as there is a lot of work to 
do to port it e.g. to Qt5/KF5. I think it wouldn't be good when I sent 
every review request to kcd... :)

So where are the patches that need review?

https://reviewboard.kde.org/r/121814/ - bugreport from ubuntu
https://reviewboard.kde.org/r/121990/ - krazy2 fixes
https://reviewboard.kde.org/r/121986/ - bugreport from kdebugs


Thx,
Christian

State of kdesvn?

[Christian Ehrlicher]

Hello,

I recently tried to fix some bugs in kdesvn but got stuck because there 
is nobody who can review my patches. Rajko Albrecht as the original 
maintainer stopped the development of kdesvn sometime in 2013 and the 
other two project members (David Faure and Christophe Giboudeaux) have 
no time to dig deep into the subversion api for a proper review.
Therefore I'm asking for help and suggestions how to go further. Maybe 
there are some people on this list who are still working with subversion 
and kdesvn and are interested in the further development of kdesvn.


Cheers,
Christian