Request for review: Grepview plugin rewrite

Matt Rogers mattr at kde.org
Fri Aug 27 13:53:41 UTC 2010


On Fri, Aug 27, 2010 at 7:29 AM, Aleix Pol <aleixpol at kde.org> wrote:
> 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).

kdelibs already depends on pcre for kjs (because pcre is much faster).
So it's at least already installed on a system.

> - 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
>>

--
Matt




More information about the KDevelop-devel mailing list