[Kde-pim] Review Request 114407: OccurrenceIterator: ensure that incidences with recurrences ID and a base incidence outside start/end range are included in iterator

[Christian Mollekopf]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114407/#review45552
-----------------------------------------------------------



kcalcore/occurrenceiterator.h
<http://git.reviewboard.kde.org/r/114407/#comment32513>

    The OccurrenceIterator should work with non-recurring incidences and is used like that in Korganizer (I think).



kcalcore/occurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32514>

    const KDateTime &



kcalcore/occurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32515>

    const KDateTime &



kcalcore/occurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32508>

    While the recurrence algorithm as defined in the RFC doesn't take the status into account, this piece of code is about ignoring all occurrences that are canceled in order to not show it in the calendar. Therefore I think it makes sense to keep this test at the beginning of the loop. 



kcalcore/occurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32509>

    Always use curly braces, also for oneline statements after an if/for/...
    
    I find:
    if ( occurrenceDate < start || occurrenceDate > end )
    
    much easier to read



kcalcore/occurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32510>

    whitespace



kcalcore/occurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32511>

    The OccurrenceIterator should work with non-recurring incidences and is used like that in Korganizer (I think).



kcalcore/tests/testoccurrenceiterator.cpp
<http://git.reviewboard.kde.org/r/114407/#comment32512>

    This seems to check that exceptions that are outside of the specified range are ignored. Can you give it a more descriptive name and test the range on both sides?


- Christian Mollekopf


On Dec. 11, 2013, 3:45 p.m., Justus Matthiesen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114407/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2013, 3:45 p.m.)
> 
> 
> Review request for KDEPIM and KDEPIM-Libraries.
> 
> 
> Bugs: 328585
>     http://bugs.kde.org/show_bug.cgi?id=328585
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> Summary
> --------
> 
> offset-related changes
> (1.1) cancelled (recurring) events contribute to offsets
> (1.2) can now have a non-zero initial offset due to an exception just before start of range
> 
> range-related special cases
> (2.1) skipping over exceptions whose original start date was in specified start/end range but new one is no longer
> (2.1) including exception whose original start date was out of range but new one is inside
> 
> This diff was originally a series of 10 commits; see branch KDE/4.11 in repository git://gitorious.org/kdepimlibs/kdepimlibs.git for details (or https://gitorious.org/kdepimlibs/kdepimlibs ).
> 
> 
> Diffs
> -----
> 
>   kcalcore/occurrenceiterator.h 5927ad587922af9fd25d6a04af8bc298d37554ab 
>   kcalcore/occurrenceiterator.cpp d3bccf5ed829602f84011c2ada892c6266ff01ba 
>   kcalcore/tests/testoccurrenceiterator.h e79e5cdb8a0947380b818d45484f6eeb1a136026 
>   kcalcore/tests/testoccurrenceiterator.cpp d07ccd939e437c7047bf22dab986300bf20824ad 
> 
> Diff: http://git.reviewboard.kde.org/r/114407/diff/
> 
> 
> Testing
> -------
> 
> All unit test pass; my own calendar file seems to functions without any issues; and test case [1] attached to bug report #328589 now renders correctly.
> 
> [1] http://bugsfiles.kde.org/attachment.cgi?id=84005
> 
> 
> Thanks,
> 
> Justus Matthiesen
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/