Request for review: Grepview plugin rewrite

Aleix Pol aleixpol at kde.org
Fri Aug 27 12:29:32 UTC 2010


Well, before having tried it I have been browsing through the code for a
while and I have few comments.

- Please explain a little the background, why do you use PCRE instead of
QRegExp? it adds a dependency and I'm not sure if it's such a big advantage
(I don't know, but there has to be a reason).
- You have been adding the get* prefix to some methods, we generally don't
use that to adapt to Qt naming, so don't add those.
- Don't use singleShot for delaying slot processing, it's preferred to use
QMetaObject::invokeMethod with Qt::QueuedConnection type
- You probably want to use the standard OutputModel you use for storing
output. That way you will get some optimizations to make displaying a little
faster, I see that you're doing some messy stuff with the model though, so
we would see.

On Thu, Aug 26, 2010 at 9:22 PM, Aleix Pol <aleixpol at kde.org> wrote:

> yup
>
>
> On Thu, Aug 26, 2010 at 7:38 PM, Syron <mr.syron at googlemail.com> wrote:
>
>>  > well then, at least please merge the current kdevelop so that I can
>> recompile against kdevplatform master.
>>
>> So you mean...?
>> $ git checkout grepview2
>> $ git merge master
>> $ git push myclone grepview2
>>
>>
>> -- Syron
>>
>> --
>> KDevelop-devel mailing list
>> KDevelop-devel at kdevelop.org
>> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20100827/5b2fa1e4/attachment.html>


More information about the KDevelop-devel mailing list