Review request: control flow graph plugin

Sandro Andrade sandro.andrade at gmail.com
Mon Oct 19 17:14:32 UTC 2009


Thanks Andres,

Now, after Gluon Sprint + Qt Dev Days, I have some free time to make
this adjustments.

Sandro

On Sat, Oct 17, 2009 at 3:02 PM, Andreas Pakulat <apaku at gmx.de> wrote:
> On 06.10.09 15:36:58, Sandro Andrade wrote:
>> On Tue, Oct 6, 2009 at 4:14 AM, Andreas Pakulat <apaku at gmx.de> wrote:
>>
>> > On 05.10.09 19:14:02, Sandro Andrade wrote:
>> > > Hi guys,
>> > >
>> > > Any suggestions for the control flow graph plugin ?
>> >
>> > TBH: I didn't spend any time on kdevelop in the last 2 weeks.
>> > Additionally I still can't compile the plugin because kgraphviewer
>> > doesn't build against libgraphviz 2.20 (it apparently needs 2.22). I've
>> > asked the author already, but seems he didn't have time yet to look into
>> > it.
>> >
>>
>> Hi Andreas, I'll check this by myself.
>>
>>
>> >
>> > I'll try to do a review nonetheless in the next couple of days.
>>
>> Ok, thanks !
>
> Sooooo first round (no in-depth stuff, but I've looked across the files
> a bit):
>
> - indentation is mixing space and tabs, we usually use space-only, would
>  be nice if you could change that either way so its consistent
>
> - DUChainControlFlow::selectionIs seems to be a rather bad function
>  name
>
> - line 54/55 in controlflowgraphview.cpp look like they should be a
>  normal function call, not s signal/slot connection? Also if you want a
>  read-write part you should use the create<KParts::ReadWritePart>
>
> - deregistering the toolview should be done in the unload method, as
>  plugin instances are never deleted (until end of the program)
>
> - the toolbutton signal/slot connection in controlflowgraphview would
>  benefit from a QSignalMapper to hide the (IMHO) ugly use of
>  sender()+object names
>
> - The license headers mention both LGPL and GPL (first paragraph says
>  LGPL, second says license text of GNU GPL). Also there's no mentioning
>  of LGPLv2+ etc. (i.e. is it v2, v3, v2+v3 or v2 and any later)
>
> Thats all folks. Obviously I have no idea about the duchain-related
> code, so can't comment on that :)
>
> I'll see wether I can find out why its crashing here tomorrow.
>
> Andreas
>
> --
> After your lover has gone you will still have PEANUT BUTTER!
>
> --
> KDevelop-devel mailing list
> KDevelop-devel at kdevelop.org
> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>



-- 
Sandro Santos Andrade
--------------------------------------------------------
http://sandroandrade.wordpress.com
http://liveblue.wordpress.com
Distributed Systems Laboratory (LaSiD)
Computer Science Department (DCC)
Federal University of Bahia - Brazil
KDE developer - KDevelop project




More information about the KDevelop-devel mailing list