Moving Plasma Media Center to extragear

David Edmundson david at davidedmundson.co.uk
Fri May 24 23:46:44 BST 2013


>From my side (as someone who reviewed this) I'm happy for this to go
to extragear too.

David

On Tue, May 21, 2013 at 5:37 AM, Sinny Kumari <ksinny at gmail.com> wrote:
> Hi,
>
> Any news on this? Does PMC look ok now from your side so that we can move it
> to extragear ?
>
> If not, please let us know what all changes we still need to do.
>
> Thanks
>
>
> On Mon, May 13, 2013 at 8:18 PM, Shantanu Tushar Jha <shantanu at kde.org>
> wrote:
>>
>> 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
>
>
>
>
> --
> http://www.sinny.in




More information about the kde-core-devel mailing list