Moving Plasma Media Center to extragear

[Shantanu Tushar Jha]

Hi David,

Thanks for the points, we have fixed everything you mentioned. For the
d-pointer and SONAME issues, I understand we should be more vigilant from
now on and take care of these things. While the mistake in the first
release is undoable, I think its better we fix things right now, as we
don't really have more than a couple of people using the libs outside
mediacenter.
In a summary, we've added d-pointers wherever there is a public header and
updated SONAME so the release 1.1 will have 1.1 set as SONAME.

Also, the krazy issues are fixed as well, alongwith support for translation
with lot of help :)


On Mon, Apr 22, 2013 at 5:05 PM, David Edmundson <david at davidedmundson.co.uk
> wrote:

> Summary:
>
> Few minor comments. Nothing particularly exciting.
> All in all I was fairly impressed with the code, it's neat and tidy
> and you've had 2 releases which is the right approach to getting in
> Extragear
>
> Assuming you've addressed Albert's comments, +1 from me on this.
>
> Comments
> ---
>
>
> FilterPlaylistModel::setFilterString
>
> don't call beginResetModel() instead call invalidateFilter()
>
> PlaylistModel::PlaylistModel
>
> avoid creating a new KStandardDirs instance. Use KGlobal::dirs()
>
> LocalVideosModel::m_pendingThumbnails
> you're storing a QModelIndex for the thumb being fetched. Always store
> QPersistantModelIndexes in case the model changes
>
> BackendsModel and PlaylistsModel should be using a d-pointer, as they
> are in the library and the headers are installed. (Assuming this is
> meant to be a public API. The fact that you've already called the
> release 1.0, to me implies it should be, people assume a lot from a
> version number). Obviously fixing this will break the ABI... so I'm
> not sure what you want to do.
>
> Related, make sure you are setting your .so version number correctly.
> It's on 0.9.. which could mean you've been ABI stable since 0.9 (which
> is great) or it could mean you set it the first release and forgot
> about it. I've not checked the git log to confirm either way.
>
> SubtitleProvider
> whitespace is all over the place
>
>
> In general:
>
> Not enough comments, this applies to almost every software project
> ever made, so this isn't a blocker - but as the maintainer I suggest
> you keep an eye out for it, and start telling people to add comments
> in review when they write something non-trivial.
>
> I'm not sure the last release should have gone in "stable" on
> download.kde.org given as it wasn't in extragear yet. It's too late to
> change now, but I wanted to draw attention on the list to it. I think
> we maybe need to define that better.
>



-- 
Shantanu Tushar    (UTC +0530)
http://www.shantanutushar.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130513/d7bf61c1/attachment.htm>