Thanks to all of you for valuable feedback. With your positive response, I have made request to sysadmin now https://bugs.kde.org/show_bug.cgi?id=317649 .
On Sat, May 25, 2013 at 4:16 AM, David Edmundson <[email protected] > wrote: > 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 <[email protected]> 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 <[email protected]> > > 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 > >> <[email protected]> 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 > -- http://www.sinny.in
