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
