<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="https://git.reviewboard.kde.org/r/115695/">https://git.reviewboard.kde.org/r/115695/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On February 19th, 2014, 11:06 a.m. CET, <b>Aleix Pol Gonzalez</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="https://git.reviewboard.kde.org/r/115695/diff/3/?file=243841#file243841line66" style="color: black; font-weight: bold; text-decoration: underline;">src/knotificationmanager.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 3)

    </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; ">KNotificationManager *KNotificationManager::self()</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">66</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">//     addPlugin(new NotifyBySound(this));</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;">This will be done in the future? Maybe it would be better to commit when there are no regressions?</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;">Yes it will..again, as said in the description - "but as the Alpha2 is quite soon, I'd like to start the general review asap so some more changes can be done if needed."

...so this is more about the general change rather than the final code ;)</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On February 19th, 2014, 11:06 a.m. CET, <b>Aleix Pol Gonzalez</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="https://git.reviewboard.kde.org/r/115695/diff/3/?file=243841#file243841line180" style="color: black; font-weight: bold; text-decoration: underline;">src/knotificationmanager.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 3)

    </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; ">KNotificationManager *KNotificationManager::self()</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">168</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">//</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;">?</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;">Yet once again the description xD - "it's full of ... FIXMEs to indicate what needs doing"</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On February 19th, 2014, 11:06 a.m. CET, <b>Aleix Pol Gonzalez</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="https://git.reviewboard.kde.org/r/115695/diff/3/?file=243849#file243849line37" style="color: black; font-weight: bold; text-decoration: underline;">src/notifybypopupgrowl.h</a>
    <span style="font-weight: normal;">

     (Diff revision 3)

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



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">NotifyByPopupGrowl</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;">Is growl still the thing for MacOS X?</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;">I'm not sure to be honest...I've heard they reworked it and I've no idea if they still support Growl...also Growl used to be on Windows afair, but have no idea about it there either...I'd personally be in favor of dropping this as I have no way to test</pre>
<br />




<p>- Martin</p>


<br />
<p>On February 13th, 2014, 11:14 a.m. CET, Martin Klapetek wrote:</p>








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

<div>Review request for kde-workspace, KDE Frameworks, Plasma, and Sune Vuorela.</div>
<div>By Martin Klapetek.</div>


<p style="color: grey;"><i>Updated Feb. 13, 2014, 11:14 a.m.</i></p>









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


<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;">This patch merges KNotify daemon into KNotificationManager to have less daemons running and less dbus traffic. The patch is not yet finished (and for now it's full of QDebugs, that will all be removed and FIXMEs to indicate what needs doing), but as the Alpha2 is quite soon, I'd like to start the general review asap so some more changes can be done if needed.

Now it's KNotificationManager that handles the KNotifyPlugin-s and hands them the notification directly. KNotifyConfig was repurposed a bit, now it serves mostly just as the .notifyrc wrapper, all the rest is reused from the KNotification object. There are some changes in the KNotification API - id() and appName() are now exposed to public and "slotReceivedId(int)" is now also public so that KNotificationManager can directly give it an id. I'd like to rename this and make it a non-slot. It's not the DBus/Galago server ID anymore, that is handled in NotifyByPopup which is responsible for communicating with the galago server (all the methods there were renamed to actually have *galago* in the name so it's clear), therefore the mapping of DBus/Galago Server ids is managed only there as it is actually only needed here. KNotitification::id() is assigned by the KNotificationManager and it's a simple increasing counter.

The UI/Plasmoid changes will come next - basically the plan is to put only the "Persistent" notifications in the notifications history.</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;">Works perfectly with both plasma notifications and kpassivepopup.</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>src/knotifyconfig.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/knotifyconfig.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/knotifyplugin.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/knotifyplugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/notifybypopup.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/notifybypopup.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/notifybypopupgrowl.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/notifybypopupgrowl.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>CMakeLists.txt <span style="color: grey">(63ebf71)</span></li>

 <li>src/CMakeLists.txt <span style="color: grey">(a81b913)</span></li>

 <li>src/knotification.h <span style="color: grey">(00554ba)</span></li>

 <li>src/knotification.cpp <span style="color: grey">(5d7405b)</span></li>

 <li>src/knotificationmanager.cpp <span style="color: grey">(a4d0dfa)</span></li>

 <li>src/knotificationmanager_p.h <span style="color: grey">(81d962d)</span></li>

</ul>

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







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








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