Review request: control flow graph plugin

[Andreas Pakulat]

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!