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








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On June 16th, 2012, 4:15 p.m., <b>Lamarque Vieira Souza</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/105278/diff/1/?file=68027#file68027line408" style="color: black; font-weight: bold; text-decoration: underline;">kdeui/actions/kaction.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

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

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KAction::setAuthAction(KAuth::Action *action)</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">408</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="c1"><span class="hl">//</span>delete d->authAction;</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">408</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">delete</span> <span class="n">d</span><span class="o">-></span><span class="n">authAction</span><span class="p">;</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, commit 3d789c9dcda0179aac40e2bcf58df06cccf84ed5 is the one that commented this line, but Dario did not give any reason why not delete the action.</pre>
 </blockquote>



 <p>On June 23rd, 2012, 6:18 p.m., <b>Lamarque Vieira Souza</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think you should have asked Dario why he did this change in the first place, also you should not commit a patch without a "ship it" from another developer.</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Indeed. The fact I didn't comment the change is not a good excuse for reverting it without asking. However, since the action can actually be used outside the scope of the button by the application and in general the action is never "parented" to the button itself, the application should track it and delete it accordingly. Moreover, it's possible an application initializes a KAction on the stack, causing quite a decent amount of troubles when deleting it from the button. In general, the UI component should never be responsible for its deletion. In frameworks this will be more obvious as actions will be implicitly shared objects.

So please revert your commit.</pre>
<br />




<p>- Dario</p>


<br />
<p>On June 16th, 2012, 3:37 p.m., Jaime Torres Amate wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for kdelibs.</div>
<div>By Jaime Torres Amate.</div>


<p style="color: grey;"><i>Updated June 16, 2012, 3:37 p.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <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;">1. Do not want to check m_startDate.isValid() twice and m_endDate.isValid() none.
2. why do not want to delete d->authAction if it is nulled after that.
3. Is really the code after the break unwanted code?
4. if ok is not initialized, sometimes while(ok) could do nothing.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <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;">6 months or more with it locally.</pre>
  </td>
 </tr>
</table>




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

 <li>kdecore/date/kcalendarera.cpp <span style="color: grey">(0a21e37)</span></li>

 <li>kdeui/actions/kaction.cpp <span style="color: grey">(309cf82)</span></li>

 <li>kio/kfile/kpropertiesdialog.cpp <span style="color: grey">(feb0c9e)</span></li>

 <li>sonnet/unicode/parseucd/parseucd.cpp <span style="color: grey">(1c9b90e)</span></li>

</ul>

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




  </td>
 </tr>
</table>








  </div>
 </body>
</html>