[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: kdesupport/phonon
From:       Matthias Kretz <kretz () kde ! org>
Date:       2008-08-21 9:36:07
Message-ID: 200808211136.12628.kretz () kde ! org
[Download RAW message or body]


On Wednesday 20 August 2008 23:50:29 Helio Chissini de Castro wrote:
> - Qtfied phonon-xine.

Hi Helio,

reviewing that patch, here's what's still left to do:

o replace qDebug() calls with a custom function that allows to switch debug 
messages off via an environment variable (since we don't have kdebugdialog 
available anymore)

o qFatal is not the correct replacement for kError

o and qWarning is not the correct replacement for kFatal

o QT_(BEGIN|END)_NAMESPACE is not meant for code outside of Qt

o please look for a way to make the KDE platform plugin provide the actual 
QIcon for the cases where the KIcon objects were replaced with QString

o you simply deleted all config settings? Please get them back using QSettings 
and provide a migration script

o you removed some i18n calls, not replacing it with one of Qt's translate 
functions, please use QObject::tr or the global translate function instead of 
i18n

o you removed some comments (e.g.
-  "execution of all clients, and low latency operation.</p></html>"),
-  /*icon name */"audio-backend-jack", outputPlugins[i]);
+  "execution of all clients, and low latency operation.</p></html>","audio-
backend-jack", outputPlugins[i] );
). Please put the comments back.

o And please restore the kdelibs coding style where you changed it.

o why did you comment out this?
-extern const char Error__off_t_needs_to_have_64_bits[sizeof(off_t) == 8 ? 1 : 
-1];
+//extern const char Error__off_t_needs_to_have_64_bits[sizeof(off_t) == 8 ? 1 
: -1];

I guess because you made off_t 32 bit (you're working on a 32 bit system?). 
The check was there for a reason.

o in volumefadereffet.cpp you unnecessarily removed a kError line

o you removed all i18n calls in volumefader_plugin.c, even I18N_NOOP should be 
possible with Qt's system

o you broke loading the volume fader plugin by removing a K from KVolumeFader

o you reverted one of my last commits which made it cache the memcpy benchmark 
results (now it'll benchmark all its memcpy methods on startup again)

o this can't work: tr( QString( "Cannot open media data at '<i>%1</i>'" ).arg( 
m_mrl.constData()))

o you probably broke some URLs:
-    gaplessSwitchTo(url.url().toUtf8());
+    gaplessSwitchTo(url.toString().toUtf8());
use QUrl::toEncoded instead



-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/
MatthiasKretz@gmx.net, kretz@kde.org,
Matthias.Kretz@urz.uni-heidelberg.de


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]