[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request 110475: Fixed::Bug 319626 - Can't remove an another song from playlist without st
From:       Shantanu Tushar Jha <shantanu () kde ! org>
Date:       2013-05-17 14:23:05
Message-ID: CABQ4Km-mez0VGQ2G-Qaz3_x_CJBYDcc7gRdBzpVioUh2iYWc1Q () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Ok I will commit it. About the comment, if you go by that reasoning, the
code will become full of unnecessary comments. So unless absolutely
necessary, you don't want to do that. If someone wants it back, there is
git ;)


On Thu, May 16, 2013 at 10:52 PM, Akshay Ratan <akshayratan@gmail.com>wrote:

> Hi,
> Yes I think it would be perfect if you commit it for me. I dont yet
> have KDE Developers Right.
> 
> Also, my reason for commenting the line was if in future somebody wants to
> code some added feature regarding the playlist, the line might be of some
> help. Anyways, should I remove it if you say ?
> 
> Cheers,
> Akshay Ratan
> 
> On Thu, May 16, 2013 at 10:45 PM, Shantanu Tushar <shantanu@kde.org>wrote:
> 
> > This is an automatically generated e-mail. To reply, visit:
> > http://git.reviewboard.kde.org/r/110475/
> > 
> > Ship it!
> > 
> > Looks good. Should I commit this for you?
> > 
> > 
> > mediaelements/playlist/PlaylistDelegate.qml<http://git.reviewboard.kde.org/r/110475/diff/1/?file=144360#file144360line59> \
> > (Diff revision 1)    58
> > 
> > visible: listViewItem.ListView.isCurrentItem
> > 
> > 59
> > 
> > // visible: listViewItem.ListView.isCurrentItem
> > 
> > don't use comments, just remove the line
> > 
> > 
> > - Shantanu
> > 
> > On May 16th, 2013, 5:07 p.m. UTC, Akshay Ratan wrote:
> > Review request for Plasma, Marco Martin, Shantanu Tushar, and Sinny
> > Kumari.
> > By Akshay Ratan.
> > 
> > *Updated May 16, 2013, 5:07 p.m.*
> > Description
> > 
> > Fixed bug 319626 ( https://bugs.kde.org/show_bug.cgi?id=319626) . Now, in the \
> > media playlist, "remove" sign is shown on every song , so the user has to just \
> > click that "sign" instead of clicking the song which earlier stopped the media. 
> > So basically now the current media is not stopped when deleting an item from a \
> > playlist. 
> > 
> > Testing
> > 
> > Yes
> > 
> > *Bugs: * https://bugs.kde.org/show_bug.cgi?id=319626<http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=319626>
> >  Diffs
> > 
> > - mediaelements/playlist/PlaylistDelegate.qml (449a0aa)
> > 
> > View Diff <http://git.reviewboard.kde.org/r/110475/diff/>
> > 
> > _______________________________________________
> > Plasma-devel mailing list
> > Plasma-devel@kde.org
> > https://mail.kde.org/mailman/listinfo/plasma-devel
> > 
> > 
> 
> 
> --
> Akshay
> 



-- 
Shantanu Tushar    (UTC +0530)
http://www.shantanutushar.com


[Attachment #5 (text/html)]

<div dir="ltr"><div>Ok I will commit it. About the comment, if you go by that \
reasoning, the code will become full of unnecessary comments. So unless absolutely \
necessary, you don&#39;t want to do that. If someone wants it back, there is git \
;)<br> </div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, \
May 16, 2013 at 10:52 PM, Akshay Ratan <span dir="ltr">&lt;<a \
href="mailto:akshayratan@gmail.com" \
target="_blank">akshayratan@gmail.com</a>&gt;</span> wrote:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Hi, <div>    Yes I think it would be perfect if you commit it \
for me. I dont yet have KDE Developers Right. </div><div> <br></div><div>Also, my \
reason for commenting the line was if in future somebody wants to code some added \
feature regarding the playlist, the line might be of some help. Anyways, should I \
remove it if you say ? </div> <div><br></div><div>Cheers,</div><div>Akshay \
Ratan<br><br><div class="gmail_quote"><div><div class="h5">On Thu, May 16, 2013 at \
10:45 PM, Shantanu Tushar <span dir="ltr">&lt;<a href="mailto:shantanu@kde.org" \
target="_blank">shantanu@kde.org</a>&gt;</span> wrote:<br>

</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex"><div><div class="h5">



 <div>
  <div style="font-family:Verdana,Arial,Helvetica,Sans-Serif"><div>
   <table style="border:1px #c9c399 solid" bgcolor="#f9f3c9" cellpadding="8" \
width="100%">  <tbody><tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/110475/" \
target="_blank">http://git.reviewboard.kde.org/r/110475/</a>  </td>
    </tr>
   </tbody></table>
   <br>



 </div><p>Ship it!</p>



 <pre style="white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">Looks \
good. Should I commit this for you?</pre>  <br>







<div>




<table bgcolor="white" border="0" width="100%">
 <thead>
  <tr>
   <th colspan="4" style="border-bottom:1px solid #c0c0c0;font-size:9pt;padding:4px \
8px;text-align:left" bgcolor="#F0F0F0">  <a \
href="http://git.reviewboard.kde.org/r/110475/diff/1/?file=144360#file144360line59" \
style="text-decoration:underline;font-weight:bold" \
target="_blank">mediaelements/playlist/PlaylistDelegate.qml</a>  <span \
style="font-weight:normal">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th style="border-right:1px solid #c0c0c0" align="right" \
bgcolor="#e9eaa8"><font>58</font></th>  <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size:8pt;line-height:140%;margin:0">            <span>visible:</span> \
<span>listViewItem</span><span>.</span><span>ListView</span><span>.</span><span>isCurrentItem</span></pre>


</td>
    <th style="border-left:1px solid #c0c0c0;border-right:1px solid #c0c0c0" \
align="right" bgcolor="#e9eaa8"><font>59</font></th>  <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size:8pt;line-height:140%;margin:0">           \
<span><span>//</span> visible: listViewItem.ListView.isCurrentItem</span></pre></td>  \
</tr>

 </tbody>

</table>

<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">don&#39;t \
use comments, just remove the line</pre> </div>
<br>



<p>- Shantanu</p><div>


<br>
<p>On May 16th, 2013, 5:07 p.m. UTC, Akshay Ratan wrote:</p>








</div><table style="background-image:url(&#39;&#39;);background-repeat:repeat-x;border:1px \
black solid" bgcolor="#fefadf" cellpadding="8" cellspacing="0" width="100%">  \
<tbody><tr>  <td><div>

<div>Review request for Plasma, Marco Martin, Shantanu Tushar, and Sinny \
Kumari.</div> <div>By Akshay Ratan.</div>


</div><p style="color:grey"><i>Updated May 16, 2013, 5:07 p.m.</i></p><div>






<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1>
 <table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%">  <tbody><tr>
  <td>
   <pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">Fixed \
bug 319626 ( <a href="https://bugs.kde.org/show_bug.cgi?id=319626" \
target="_blank">https://bugs.kde.org/show_bug.cgi?id=319626</a>) . Now, in the media \
playlist, &quot;remove&quot; sign is shown on every song , so the user has to just \
click that &quot;sign&quot; instead of clicking the song which earlier stopped the \
media.

So basically now the current media is not stopped when deleting an item from a \
playlist.  </pre>
  </td>
 </tr>
</tbody></table>


<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%">  <tbody><tr>
  <td>
   <pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">Yes</pre>
  </td>
 </tr>
</tbody></table>



<div style="margin-top:1.5em">
 <b style="color:#575012;font-size:10pt;margin-top:1.5em">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=319626" \
target="_blank">https://bugs.kde.org/show_bug.cgi?id=319626</a>


</div>


<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">

 <li>mediaelements/playlist/PlaylistDelegate.qml <span \
style="color:grey">(449a0aa)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/110475/diff/" style="margin-left:3em" \
target="_blank">View Diff</a></p>







  </div></td>
 </tr>
</tbody></table>








  </div>
 </div>


<br></div></div>_______________________________________________<br>
Plasma-devel mailing list<br>
<a href="mailto:Plasma-devel@kde.org" target="_blank">Plasma-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/plasma-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br> \
<br></blockquote></div><span class="HOEnZb"><font color="#888888"><br><br \
clear="all"><div><br></div>-- <br>Akshay </font></span></div>
</blockquote></div><br><br clear="all"><br>-- <br>Shantanu Tushar    (UTC \
+0530)<br><a href="http://www.shantanutushar.com" \
target="_blank">http://www.shantanutushar.com</a> </div>



_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic