<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://svn.reviewboard.kde.org/r/5704/">http://svn.reviewboard.kde.org/r/5704/</a>
</td>
</tr>
</table>
<br />
<div>
<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://svn.reviewboard.kde.org/r/5704/diff/1/?file=40339#file40339line70" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h</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; ">public:</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">70</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * @deprecated</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">add a hint about what should be used instead.</pre>
</div>
<br />
<div>
<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://svn.reviewboard.kde.org/r/5704/diff/1/?file=40339#file40339line84" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h</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; ">public:</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">84</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * @deprecated</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">add a hint about what should be used instead.</pre>
</div>
<br />
<div>
<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://svn.reviewboard.kde.org/r/5704/diff/1/?file=40339#file40339line196" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h</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; ">public:</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">196</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * @deprecated</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">add a hint about what should be used instead.</pre>
</div>
<br />
<div>
<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://svn.reviewboard.kde.org/r/5704/diff/1/?file=40339#file40339line204" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h</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; ">public:</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">204</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//KDE5 make virtual?</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">just as a sidenote: If in KDE5 you would make methods in this class virtual it would mean that one could implement custom calendarsystems.
However, these would then not be reflected in the enum. Thus, they could not be managed in the same way, i.e. listed via calendarSystemList() and so on.</pre>
</div>
<br />
<div>
<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://svn.reviewboard.kde.org/r/5704/diff/1/?file=40356#file40356line759" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/localization/klocale.h</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; ">public:</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">759</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * @since 4.6</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">How about a hint to the methods that use the enum?</pre>
</div>
<br />
<p>- Sebastian</p>
<br />
<p>On October 28th, 2010, 12:56 p.m., John Layt wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 kdelibs.</div>
<div>By John Layt.</div>
<p style="color: grey;"><i>Updated 2010-10-28 12:56:04</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 is related to the KLocalizedDate review 5692. I realised in that review that KLocalizedDate should now be the only api people use to localize dates, and so rather than exactly matching the old KCalendarSystem api I could make some changes to simplify / improve the api. (The bits matching the QDate api won't change).
As part of this, I want to introduce an enum for CalendarSystem to replace the current QString Calendar Type, e.g. replace "hebrew" with KLocale::HebrewCalendar. This is obviously clearer, safer and less error prone.
Note that I had to put the enum in KLocale rather than KCalendarSystem as the KCalendarSystem api already uses KLocale enums in the header, so I can't use any KCalendarSystem enums in the KLocale api and header (you can't forward-declare enums).
The awkward part was naming the enums, particularly the default QDate hybrid Julian/Gregorian system. I chose to call this KLocale::QDateCalendar rather than KLocale::GregorianCalendar which I've used for the proper one. Any better suggestions welcome. I've also renamed the Hijri to IslamicCivilCalendar to distinguish from the lunar calendar. I'll probably rename the derived calendar classes to match the enum, which should be BC safe as they are not exported, but I'll do that later to save cluttering up the diff. [Random thought: do we even need the private classes derived from the base class anymore, the derived private d_ptr class should be enough?]
You probably don't want to look too closely at the KCalendarSystem spaghetti, you'll go cross-eyed, the KLocale is of more interest :-)</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;">All existing unit tests pass, new unit tests written. (Note, ignore the KLocalizedDate tests below, it's just the overlap from the other review, they will be committed separately.)</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>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemcoptic.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemcopticprivate_p.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemethiopian.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorian.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorianproleptic.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorianprolepticprivate_p.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhebrew.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhijri.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemindiannational.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemjalali.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemjapanese.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemjulian.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemminguo.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemprivate_p.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kcalendarsystemthai.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/localization/klocale.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/localization/klocale.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/localization/klocale_kde.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/localization/klocale_p.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/tests/kcalendartest.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/tests/kcalendartest.cpp <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/tests/klocaletest.h <span style="color: grey">(1190608)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/tests/klocaletest.cpp <span style="color: grey">(1190608)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5704/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>