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

List:       kde-devel
Subject:    Re: Several new utilities for KDE: KLook and StackFolder
From:       David Edmundson <david () davidedmundson ! co ! uk>
Date:       2012-05-07 17:15:34
Message-ID: CAGeFrHBt1WWOi4YutHooOe+g-X-B8UjTN1DA15=-oyb1c_cRMQ () mail ! gmail ! com
[Download RAW message or body]

Good to see these merged into playground!

Some things we would need to proposing for KDE extragear/core
- Watch your coding style
  http://techbase.kde.org/Policies/Kdelibs_Coding_Style
  In particular:
    Always use braces
    Don't call classes "MySomething". As the "My" isn't very helpful
    Make sure the files actually match the names of the class.
audio.h, video.h, text.h
    MyAudio::~MyAudio() use m_mediaObject->deleteLater(); instead of
delete (it's safer). Alternatively set a parent on it in the
constructor and don't manually delete it.
   Technically your whitespace is off in many ways too, but that's
being pedantic.

- Long term rendering of audio/text/video/images etc in QML needs to
be moved out of there and into somewhere more generic (some sort of
generic QML component that can display any mimetype using the
appropriate backends)

 - This has a lot of duplication with the previewer applet (which is
why we need the above to fix it). This still needs a QML port (AFAIK),
so it would be a good time to work together as it's basically the same
thing. (Frankly yours is a lot better right now)

- The way you're embedding video in QML isn't always going to work.
(it won't work with xine using xv anyway, but I think that's being
deprecated)..you don't have a lot of choice right now, but make sure
you talk to someone from KDE Multimedia about what's moving forward
with Phonon-QML.

 - Most importantly you've hardcoded it to match the theme you're
using in your distro. However on any other setup (like stock KDE) your
buttons look completely different from anything in my normal KDE
applications, and completely different from anything in Plasma. Same
for the window frame, and the icons etc. It looks absolutely great in
your screenshots where it matches the theme you're using, but on any
other setup it looks awful/out of place. Inside KDE we need to keep
consistency.

Not trying to be negative, it's a great start!

David Edmundson

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
[prev in list] [next in list] [prev in thread] [next in thread]