<div dir="ltr">Thanks to all of you for valuable feedback. With your positive response, I have made request to sysadmin now <a href="https://bugs.kde.org/show_bug.cgi?id=317649">https://bugs.kde.org/show_bug.cgi?id=317649</a> .<br>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, May 25, 2013 at 4:16 AM, David Edmundson <span dir="ltr"><<a href="mailto:david@davidedmundson.co.uk" target="_blank">david@davidedmundson.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From my side (as someone who reviewed this) I'm happy for this to go<br>
to extragear too.<br>
<span><font color="#888888"><br>
David<br>
</font></span><div><div><br>
On Tue, May 21, 2013 at 5:37 AM, Sinny Kumari <<a href="mailto:ksinny@gmail.com" target="_blank">ksinny@gmail.com</a>> wrote:<br>
> Hi,<br>
><br>
> Any news on this? Does PMC look ok now from your side so that we can move it<br>
> to extragear ?<br>
><br>
> If not, please let us know what all changes we still need to do.<br>
><br>
> Thanks<br>
><br>
><br>
> On Mon, May 13, 2013 at 8:18 PM, Shantanu Tushar Jha <<a href="mailto:shantanu@kde.org" target="_blank">shantanu@kde.org</a>><br>
> wrote:<br>
>><br>
>> Hi David,<br>
>><br>
>> Thanks for the points, we have fixed everything you mentioned. For the<br>
>> d-pointer and SONAME issues, I understand we should be more vigilant from<br>
>> now on and take care of these things. While the mistake in the first release<br>
>> is undoable, I think its better we fix things right now, as we don't really<br>
>> have more than a couple of people using the libs outside mediacenter.<br>
>> In a summary, we've added d-pointers wherever there is a public header and<br>
>> updated SONAME so the release 1.1 will have 1.1 set as SONAME.<br>
>><br>
>> Also, the krazy issues are fixed as well, alongwith support for<br>
>> translation with lot of help :)<br>
>><br>
>><br>
>> On Mon, Apr 22, 2013 at 5:05 PM, David Edmundson<br>
>> <<a href="mailto:david@davidedmundson.co.uk" target="_blank">david@davidedmundson.co.uk</a>> wrote:<br>
>>><br>
>>> Summary:<br>
>>><br>
>>> Few minor comments. Nothing particularly exciting.<br>
>>> All in all I was fairly impressed with the code, it's neat and tidy<br>
>>> and you've had 2 releases which is the right approach to getting in<br>
>>> Extragear<br>
>>><br>
>>> Assuming you've addressed Albert's comments, +1 from me on this.<br>
>>><br>
>>> Comments<br>
>>> ---<br>
>>><br>
>>><br>
>>> FilterPlaylistModel::setFilterString<br>
>>><br>
>>> don't call beginResetModel() instead call invalidateFilter()<br>
>>><br>
>>> PlaylistModel::PlaylistModel<br>
>>><br>
>>> avoid creating a new KStandardDirs instance. Use KGlobal::dirs()<br>
>>><br>
>>> LocalVideosModel::m_pendingThumbnails<br>
>>> you're storing a QModelIndex for the thumb being fetched. Always store<br>
>>> QPersistantModelIndexes in case the model changes<br>
>>><br>
>>> BackendsModel and PlaylistsModel should be using a d-pointer, as they<br>
>>> are in the library and the headers are installed. (Assuming this is<br>
>>> meant to be a public API. The fact that you've already called the<br>
>>> release 1.0, to me implies it should be, people assume a lot from a<br>
>>> version number). Obviously fixing this will break the ABI... so I'm<br>
>>> not sure what you want to do.<br>
>>><br>
>>> Related, make sure you are setting your .so version number correctly.<br>
>>> It's on 0.9.. which could mean you've been ABI stable since 0.9 (which<br>
>>> is great) or it could mean you set it the first release and forgot<br>
>>> about it. I've not checked the git log to confirm either way.<br>
>>><br>
>>> SubtitleProvider<br>
>>> whitespace is all over the place<br>
>>><br>
>>><br>
>>> In general:<br>
>>><br>
>>> Not enough comments, this applies to almost every software project<br>
>>> ever made, so this isn't a blocker - but as the maintainer I suggest<br>
>>> you keep an eye out for it, and start telling people to add comments<br>
>>> in review when they write something non-trivial.<br>
>>><br>
>>> I'm not sure the last release should have gone in "stable" on<br>
>>> <a href="http://download.kde.org" target="_blank">download.kde.org</a> given as it wasn't in extragear yet. It's too late to<br>
>>> change now, but I wanted to draw attention on the list to it. I think<br>
>>> we maybe need to define that better.<br>
>><br>
>><br>
>><br>
>><br>
>> --<br>
>> Shantanu Tushar (UTC +0530)<br>
>> <a href="http://www.shantanutushar.com" target="_blank">http://www.shantanutushar.com</a><br>
><br>
><br>
><br>
><br>
> --<br>
> <a href="http://www.sinny.in" target="_blank">http://www.sinny.in</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><a href="http://www.sinny.in" target="_blank">http://www.sinny.in</a>
</div></div>