<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/102316/">http://git.reviewboard.kde.org/r/102316/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 26th, 2011, 12:53 a.m., <b>Akarsh Simha</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/102316/diff/1/?file=31916#file31916line354" style="color: black; font-weight: bold; text-decoration: underline;">kstars/kstarsdata.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 KStarsData::setLocation( const GeoLocation &l ) {</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">354</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="o">!</span><span class="n">key</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">()</span> <span class="o">&&</span> <span class="n">m_Geo</span><span class="p">.</span><span class="n">tzrule</span><span class="p">()</span><span class="o">-></span><span class="n">equals</span><span class="p">(</span><span class="o">&</span><span class="n">Rulebook</span><span class="p">[</span><span class="n">key</span><span class="p">])</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;">Okay, I had a look into the code. I see that with the way the TimeZoneRule class is currently structured, there's no better way of doing this.
However, I would think that a better way to do this would be to have the TimeZoneRule class know what key it corresponds to, i.e. store a QString m_TZRuleKey in class TimeZoneRule which is set up in the constructor. Then modify the line in KStarsData::readTimeZoneRuleBook() that instantiates TimeZoneRule to use the new constructor with 'key' as an argument. Then, one would rewrite this bugfix as: Options::setDST( m_Geo.tzrule()->getTZRuleKey() );
You could implement that if you like. Else, I'll go ahead and sign this for you / you can go ahead and commit it, IMO.</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 committed it in master and 4.7 branch "as is". I'll work on refactoring TimeZoneRule to make the solution better (it is not the only place where the key would be useful).</pre>
<br />
<p>- Marta</p>
<br />
<p>On August 13th, 2011, 6:53 p.m., Marta Rybczyńska 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 KStars.</div>
<div>By Marta Rybczyńska.</div>
<p style="color: grey;"><i>Updated Aug. 13, 2011, 6:53 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;">This patch fixes an old bug. It turns out that DST (Daylight Savings Time) setting was not saved and that resulted with no DST correction (fallback "--" DST rule when all other location information is correct) when the program was run again. The workaround was to enter Settings->Geographic and click OK.
The patch adds change of the DST field in Option when the location is changed, that leads to the new value to be saved.
In the case of a configuration without DST saved, the configuration will be fixed next time the location is changed, also when the user follows the existing workaround.
Implementation explanation:
The patch uses a loop, because at this point we do not have the rule name, which is to be saved. Further work will be to keep the rule name in the TimeZoneRule class and refactor this code and other similar places accordingly.</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;">Tested with a config file with and without DST settings, multiple location changes etc. Works fine.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=209647">209647</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kstars/kstarsdata.cpp <span style="color: grey">(2303c5f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102316/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>