>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
