----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109991/#review31192 -----------------------------------------------------------
common/regexpcache.h <http://git.reviewboard.kde.org/r/109991/#comment23196> You do need to restore these common/regexpcache.h <http://git.reviewboard.kde.org/r/109991/#comment23195> Revert this change: the dangling _ is quite important! common/regexpcache.cpp <http://git.reviewboard.kde.org/r/109991/#comment23197> Please remove the extra space between QRegExp and & common/regexpcache.cpp <http://git.reviewboard.kde.org/r/109991/#comment23198> "Begin of pattern" is good Italian, but not so good English..."Beginning Pattern" would be better common/regexpcache.cpp <http://git.reviewboard.kde.org/r/109991/#comment23202> Why not just QString()? common/regexpcache.cpp <http://git.reviewboard.kde.org/r/109991/#comment23201> I'm slow and stupid...could you please comment what it is you are trying to do with this function? Ideally you could split it up into some smaller functions. common/regexpcache.cpp <http://git.reviewboard.kde.org/r/109991/#comment23200> Why don't you use QString::split? common/regexpcache.cpp <http://git.reviewboard.kde.org/r/109991/#comment23199> This seems like a behaviour change, rather than an optimisation? Could you split it into a separate commit (git rebase is your friend here) and comment the intended purpose of the function? Thanks for the patch! Please could you add a few comments on the design of the optimised regexp, why it is faster, how it works, and so on? It really helps us to review it faster. Also, could you please add the test cases you link to the nepomuk tests directory? If you like, submit a separate patch adding them, as a test for regexp speed and correctness would be useful in itself. - Simeon Bird On April 13, 2013, 12:56 p.m., Lukasz Olender wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109991/ > ----------------------------------------------------------- > > (Updated April 13, 2013, 12:56 p.m.) > > > Review request for Nepomuk and Vishesh Handa. > > > Description > ------- > > It's related with https://bugs.kde.org/show_bug.cgi?id=303654. > P.S. I accidentally deleted author's and license info in patch. Isolated > performance tests are also uploaded to http://www.sendspace.com/file/mkihdp > (previous link not always work). It's my first patch. > > > This addresses bug 303654. > http://bugs.kde.org/show_bug.cgi?id=303654 > > > Diffs > ----- > > common/regexpcache.h d89f968 > common/regexpcache.cpp df45277 > > Diff: http://git.reviewboard.kde.org/r/109991/diff/ > > > Testing > ------- > > > Thanks, > > Lukasz Olender > >
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
