[PATCH] API review of KCalendarSystem* and KDate* widgets

[John Layt]

Hi,

Attached are the previously discussed patches for the API review I've done on 
KCalendarSystem* and the KDate* widgets.  These patches are mainly concerned 
with getting the required API changes made before the freeze.  Further 
implementation clean-up work is ongoing and patches will be submitted later.  
Not all new methods added have supporting unit tests yet, but I'm posting the 
patches now to give people time to review them.  I plan to have the unit 
tests completed for the weekend, i.e. before final freeze in case they expose 
any issues.

API Changes to KCalendarSystem* classes:
 * API review & clean-up: 3 public methods deleted and 1 public method
   deprecated
 * Sync with QDate API changes
 * Sync with KLocale API change
 * API additions to support extended date range & validation
 * API additions to support setting calendar system for date widgets to
   display
 * Base class now offers standard implementations of most methods, rather than
   each sub-class having to implement their own
 * Sub-classes now have stubs for all base class virtual methods to protect
   against future BIC issues

API Changes to KDate* widget classes:
 * API clean-up, 1 public method changed
 * API additions to support setting calendar system for date widgets to
   display

API changes not made to KCalendarSystem* classes:
 * The setYMD() method would have been deleted instead of deprecated, except
   lxr showed over 290 usages, too many to fix now.
 * The yearString, monthString, dayString methods take a bool to indicate
   using short or long date format, these should have been replaced with
   enums/flags per API guidelines, but an lxr scan showed several hundred
   usages, too many to fix now.  (Removed enum someone had declared without
   actually using it.)

There are more changes here than originally proposed because:
 * Reviewing the BIC documentation I remembered you can't add new vitual
   methods at a later date, so needed to add them now.
 * Reviewing KLocale's usage of KCalendarSystem identified a small difference
   in terminology
 * Realising KCalendarSystem's current usage of locale was already broken and
   needed fixing anyway by adding local pointers to KLocale methods

Impact:
 * BIC broken on KCalendarSystem* due to new / deleted virtual methods
 * BIC broken on KDateWidget as setDate now returns bool instead of void
 * Source compatability maintained in KDE SVN outside KCalendarSystem* and
   KDate* widget classes
 * Where source compatability broken inside KCalendarSystem* and KDate* widget
   classes these have been fixed
 * Existing KCalendarSystem* sub-class methods not yet changed to point to new
   base class implementations, so should be no breakages in existing
   functionality
 * KDateWidget has been ported to new calendar locale methods as test bed for
   setting of calendar, but no downstream impact as no-one in KDE svn uses it

Testing:
 * I tried to run the existing unit tests, but couldn't find where the devel
   build put them, is there something I'm missing? 
 * The existing unit tests do not have full coverage of the existing code. 
   I'm in the process of writing a full set.
 * Running kfind and playing with the date picker works as expected
 * I tried changing the global calendar system in kcm to test the non-georgian
   calendars, but the kcm language module itself appears to be missing that
   section

Planned steps from here:
 * Unit tests for new methods (planned before full freeze applies)
 * Add changes to KDE4 porting guide
 * Apply astyle and other layout clean-ups
 * Convert old methods to use new methods, code clean-up, etc, provide full
   unit tests
 * apidox improvements
 * Start on plasmoid :-)

I've attached the patches and a more detailed change log.  I had moved a lot 
of methods around in the headers to better group them for apidox, and in 
the .cpp to match, but diff couldn't cope with it.  I'll try again when I do 
the astyle clean-up and just send the files in place of the diffs.

Let me know what you think.

Cheers!

John.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: api-review.tar.gz
Type: application/x-tgz
Size: 17022 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070718/2ffcaf34/attachment.bin>