<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/106912/">http://git.reviewboard.kde.org/r/106912/</a>
</td>
</tr>
</table>
<br />
<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'm not sure why am I under the impression that adjam does not use activities. Anyhow, I'll pitch in with a couple observations:
First of all, it is cool to see activities integration in rekonq (I'm sure at least mathieson has been asking for this) and I hope we can get more of this.
On the other hand, the target audience of this review is probably not related to the SLC concept so besides mentioning that you can use the SLC applet, it would be useful to give an overview of what does that do and explain how does that make Rekonq a better browser etc.
Another comment, and this applies to all reviews in the platform, not this one in particular, is the Testing Done. People seem to think it is a yes/no question or answer "Works" or "Compiles". According to ReviewBoard documentation "The Testing Done field describes how this change has been tested [...] This should cover any and all testing scenarios that have been done, in order to help reviewers feel more confident about the stability and design of the change."
Now, my review:
I patched my Rekonq master (btw, you probably want to specify that in the branch field) and was unable to configure Rekonq with my current KActivities version (6.0.0) so I updated KActivities and Nepomuk Core to latest master and was able to compile everything. Is that intended? I see the _OPTIONAL cmake macro used there, but I'm not sure why did it fail - I could test that again if you need me to. After installing the patched Rekonq, I can only report it doesn't break, but I'm not sure how to test if it is doing anything with the resources, is there a sparql query I can throw into, e.g., NepSaK to see this working in the backend?
Thanks.</pre>
<br />
<p>- David</p>
<br />
<p>On October 16th, 2012, 1:38 p.m., Ivan Čukić 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 rekonq and Andrea Diamantini.</div>
<div>By Ivan Čukić.</div>
<p style="color: grey;"><i>Updated Oct. 16, 2012, 1:38 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;">Rekonq reports the open/close document events to activity manager daemon.
Side-effect - support for Share-Like-Connect applet.
</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;">Yes, with multiple tabs and separate windows</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>CMakeLists.txt <span style="color: grey">(030f0fb)</span></li>
<li>config-kactivities.h.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(6c27376)</span></li>
<li>src/mainview.h <span style="color: grey">(89ee36e)</span></li>
<li>src/mainview.cpp <span style="color: grey">(15e0d5e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106912/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>