<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/121165/">https://git.reviewboard.kde.org/r/121165/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 13th, 2014, 9:59 a.m. CET, <b>Mehrdad Momeny</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">One thing that came to my mind today is that:
About the first item in your list: Loading more tweets instead of showing them in a page</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">It removes the old, stale <span style="color: #BA2121">"page browsing"</span> that has not worked <span style="color: #008000; font-weight: bold">for</span> the longest time. Instead, it introduces a button to <span style="color: #BA2121">"load more tweets"</span> which actually works and loads older tweets. However as of now, you have to click it TWICE <span style="color: #008000; font-weight: bold">for</span> the first time to work. After that, every addidtional click will retrieve more old tweets. They are shown on top of the newer ones which is quite unfortunate, I don<span style="border: 1px solid #FF0000">'</span>t know how to change that. (see also https<span style="color: #666666">:</span><span style="color: #408080; font-style: italic">//bugs.kde.org/show_bug.cgi?id=340795)</span>
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This will introduce a performance issue!
in fact right now we have lots of reports that Choqok is using too much memory. and this increases by showing and keeping more posts/tweets in the GUI!
So why should we drop the page browsing (I know it's not working properly now, but I mean the idea of having pages instead of adding to current list)?
Thus it might be better to keep loading them in pages, (if it's possible technically) and then the issue of loading new tweets below current list won't be there anymore.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What do you think guys?</p></pre>
</blockquote>
<p>On December 13th, 2014, 10:33 a.m. CET, <b>Ian Schwarz</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't have anything against the idea in particular, however I think that it is really cumbersome to implement and therefore strongly discouraged by the Twitter devs (https://dev.twitter.com/rest/public/timelines). You could not have "browse to page x" because you would have no idea what is on "page x". You could only have "next page", and on clicking that, get all the tweets before the oldest tweet on the page you are currently on - id--; - save this value, because we will need it to browse back to this page later. Repeat for following pages. If you want to go forward again, look the correct max_id up in the table you have been saving and query the API with that max_id string. If there are tweets added AFTER you start browsing, they will be inaccessible.</p></pre>
</blockquote>
<p>On December 13th, 2014, 10:52 a.m. CET, <b>Andrea Scarpino</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I agree with Mehrdad. We should fix the memory issue before pushing a change like 1).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ian, could you please remove 1) from this review? So we go ahead with the others two feature. The best would be to split this in 3 reviews.</p></pre>
</blockquote>
<p>On December 20th, 2014, 11:36 a.m. CET, <b>Andrea Scarpino</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Status? Ian are you working to split 2) and 3) from this? Please let we know.</p></pre>
</blockquote>
<p>On December 20th, 2014, 8:04 p.m. CET, <b>Ian Schwarz</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I will try to split it, however I can't do it today. Maybe in a few days.</p></pre>
</blockquote>
<p>On December 20th, 2014, 8:07 p.m. CET, <b>Ian Schwarz</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I caught a bug and couldn't do it yet. I'll try to get it done Monday or Tuesday. Sorry to keep you waiting.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No problem at all for the wait! It's nice to know that you are on it. Thanks!</p></pre>
<br />
<p>- Andrea</p>
<br />
<p>On December 12th, 2014, 9:24 p.m. CET, Ian Schwarz wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Choqok.</div>
<div>By Ian Schwarz.</div>
<p style="color: grey;"><i>Updated Dec. 12, 2014, 9:24 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
choqok
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch does 3 things:</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It removes the old, stale "page browsing" that has not worked for the longest time. Instead, it introduces a button to "load more tweets" which actually works and loads older tweets. However as of now, you have to click it TWICE for the first time to work. After that, every addidtional click will retrieve more old tweets. They are shown on top of the newer ones which is quite unfortunate, I don't know how to change that. (see also https://bugs.kde.org/show_bug.cgi?id=340795)</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It introduces a "follower list", similar to the "friends list" for Twitter, which contains only followers' screen names. The reason for that is that on Twitter, you can only send DM's to your FOLLOWERS instead of your "friends" (i.e. people you have followed). The git version "allows" you to send them to the people you have followed which results in mute failure and no error message. We do still need the old friends list though so we can know whom we have followed (and thus whether to display the "follow" or the "unfollow" option).</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"Show all posts by $NAME" should not use the Twitter search/tweets API, as this API will only return tweets that are less than 7 days old. Instead, use statuses/user_timeline which returns up to 3,200 tweets of any age.</p>
</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let me state that I do not expect this patch to be accepted as is or even with minor changes. What I hope instead is that people who know more about C++/Qt/Choqok look at this and go "I see what you're trying to do, but there is a better way." and then actually propose a better way. I feel I have done as much as I could to enable Choqok to make progress towards a mostly working Twitter client for Linux.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Oh, and bumping the version to 1.4.0.1 or somesuch would be good sometime in the future. Maybe this way we can get distribution/packagers to ship a more functional version of Choqok in the future. The old 1.4.0 release is horribly broken by comparison.</p></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>choqok/microblogs/twitter/twittersearch.cpp <span style="color: grey">(da85565)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapiaccount.h <span style="color: grey">(d880f14)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapiaccount.cpp <span style="color: grey">(e2efe97)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapidmessagedialog.h <span style="color: grey">(ed36a1d)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapidmessagedialog.cpp <span style="color: grey">(70092db)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapimicroblog.h <span style="color: grey">(b9dd97a)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapimicroblog.cpp <span style="color: grey">(cb666cb)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapisearch.h <span style="color: grey">(b421585)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapisearch.cpp <span style="color: grey">(b3078c4)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapisearchtimelinewidget.h <span style="color: grey">(ec40605)</span></li>
<li>choqok/helperlibs/twitterapihelper/twitterapisearchtimelinewidget.cpp <span style="color: grey">(82dc1b1)</span></li>
<li>choqok/microblogs/laconica/laconicamicroblog.cpp <span style="color: grey">(dd86db2)</span></li>
<li>choqok/microblogs/laconica/laconicasearch.h <span style="color: grey">(4bfac2d)</span></li>
<li>choqok/microblogs/laconica/laconicasearch.cpp <span style="color: grey">(4b1c9c9)</span></li>
<li>choqok/microblogs/twitter/twitterpostwidget.cpp <span style="color: grey">(f8bf945)</span></li>
<li>choqok/microblogs/twitter/twittersearch.h <span style="color: grey">(662e8d7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121165/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>