KDE/kdevelop/languages/cpp

[Andreas Pakulat]

On 19.03.09 10:12:47, David Nolden wrote:
> Am Donnerstag 19 März 2009 08:31:38 schrieb Andreas Pakulat:
> > > -  int kind =
> > > editor()->parseSession()->token_stream->kind(node->class_key); +  /*int
> > > kind = */editor()->parseSession()->token_stream->kind(node->class_key);
> >
> > Hmm, why not simply remove the variable if its not used? (IIRC there's
> > another such case in the original patch further up - which I already
> > deleted here).
> 
> But if you don't know the code, then it's always better to keep the function-
> calls,

I wasn't talking about removing the function call, I was talking about
removing the _variable_ which is now commented out.

> I think Andrew Coles does a good job at not breaking stuff with his
> warning- fixes, I've seen many less successful attempts..

No doubt about that.
 
> > Also I think you should start posting your patches to our development
> > mailinglist via reviewboard.kde.org and not commit to code you've never
> > touched before, even if its just about fixing warnings from gcc.
> I think for the really simple fixes, and if they're done properly like these, 
> it's ok. I've seen nothing dangerous in this patch, except for the unpretty 
> "#ifdef QT_DEBUG" things, so I think using review-board would be overhead in 
> this specific case.

Well, in this specific case we'd have 2 QT_DEBUGS less, possibly a fixed
possible crash for folder (and no assert) and a couple of
commented-variable-declarations that wouldn't be in the codebase. IMHO that
warrants a review on reviewboard. I do agree that all other fixes that were
done in this particular case don't need a review.

Andreas

-- 
Beware of a dark-haired man with a loud tie.