<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/112720/">http://git.reviewboard.kde.org/r/112720/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 14th, 2013, 10:27 p.m. UTC, <b>Akarsh Simha</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;">There is a problem with this change:
I think getDSSURL() is also called elsewhere where it is desirable to allow width and height to be smaller than dss_default_size (for example, it could be called via DBus). Ideally, it should not be confined to be larger than dss_default_size if called outside of KStars.</pre>
</blockquote>
<p>On September 14th, 2013, 11:19 p.m. UTC, <b>Rafal Kulaga</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;">You're obviously right. Anyway, those two ifs can and should be combined.</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;">Yeah, I guess this patch should be shipped because it makes the code cleaner. If I ever notice that something is not working as expected because of this change, and the only way is to let getDSSURL() be a little dumber, I'll go back and change things. But this is cleaner, and I agree it should be okay to commit it.
</pre>
<br />
<p>- Akarsh</p>
<br />
<p>On September 14th, 2013, 11:20 p.m. UTC, Vijay Dhameliya wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 KStars, Akarsh Simha and Rishab Arora.</div>
<div>By Vijay Dhameliya.</div>
<p style="color: grey;"><i>Updated Sept. 14, 2013, 11:20 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;">Height and width of image are being checked twice for same validity under getDSSURL function so first check is removed.
Making Wish list and Session plan table column under Observation list resizable by user for better usability. </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;">Runs correctly </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>kstars/ksutils.cpp <span style="color: grey">(d517deb)</span></li>
<li>kstars/tools/observinglist.cpp <span style="color: grey">(7eb5ab5)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112720/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>