gcc -Woverloaded-virtual

[David Faure]

On Monday 26 November 2007, Aurélien Gâteau wrote:
> David Faure wrote:
> 
> >> Maybe the opposite : something like
> >> void addJobWithoutMerge(KJob *job)
> >> 
> >> So we will use the compile error to help us to port.
> >> This is also better API than using a meaningless bool.
> > 
> > Yes, addSubjobNoMetadataMerge is a good solution.
> > We'll also find out if that's really used anywhere... I don't see any use
> > within kio, for instance... Hmm... so maybe we should first try to compile
> > KDE without that bool to find out if that's really used anywhere. Copying
> > the metadata into the subjob gives it the right window-id and
> > user-timestamp for instance, I don't know of a case where we don't want to
> > copy the metadata to the subjob.
> 
> I just ran a search for addSubjob(.*, .*) on my KDE checkout (kdesupport,
> kdelibs, kdepimlibs, kdebase and kdegraphics) and did not find any match
> except for the method implementation. Of course, one should check other
> modules as well. Still it's a good indication that removing the boolean
> parameter is probably the simplest solution.
Agreed.

> As for removeSubjob(), I found two calls to removeSubjob(job, true). Those
> are in copyjob.cpp. In the case of removeSubjob(), the default behavior is
> to not merge metadata, so it we can either:
> - add a removeSubjobAndMergeMetadata() method
> - add a separate mergeMetadata() method
Separate call sounds good. Doesn't really need a new method though, there's
already a mergeMetadata but that's for outgoing metadata (job->slave) while 
here it's about the resulting metadata (incoming, i.e. slave->job).

I would just do d->m_incomingMetaData += job->metaData() before
the removeSubjob(job) call.
And then the dynamic cast in removeSubjob can be removed altogether
(well, so the whole method is just a base class call, but maybe better keep it in case we need it later?)

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).