qtorganizer5-eds

Merge lp:~renatofilho/qtorganizer5-eds/fix-1616040 into lp:qtorganizer5-eds

  1. fix-1616040
  2. Merge into trunk
Proposed by Renato Araujo Oliveira Filho
Status: Merged
Approved by: Gustavo Pichorim Boiko
Approved revision: 110
Merged at revision: 113
Proposed branch: lp:~renatofilho/qtorganizer5-eds/fix-1616040
Merge into: lp:qtorganizer5-eds
Diff against target: 16 lines (+5/-1)
1 file modified
organizer/qorganizer-eds-engine.cpp (+5/-1)
To merge this branch: bzr merge lp:~renatofilho/qtorganizer5-eds/fix-1616040
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
Gustavo Pichorim Boiko (community) Approve
system-apps-ci-bot continuous-integration Approve
Review via email: mp+303726@code.launchpad.net

Commit message

Fix recurrence date.

Does not convert recurrence date to UTC before save it.

To post a comment you must log in.
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

PASSED: Continuous integration, rev:110
https://jenkins.canonical.com/system-apps/job/lp-qtorganizer5-eds-ci/10/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build/1279
    SUCCESS: https://jenkins.canonical.com/system-apps/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=default/280
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1279
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/1145
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/1145
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/1145
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1128/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1128
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1128/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-qtorganizer5-eds-ci/10/rebuild

review: Approve (continuous-integration)
Revision history for this message
Gustavo Pichorim Boiko (boiko) wrote :

Looks good!

review: Approve
Revision history for this message
Charles Kerr (charlesk) wrote :

LGTM, should work fine.

Only minor suggestion is we pull limitDate() 4x in a small paragraph of code.

If QOrganizerRecurrenceRule does something dumb like recalculating it each time limitDate() is called, be better call it just once and cache it in a const temporary.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'organizer/qorganizer-eds-engine.cpp'
2--- organizer/qorganizer-eds-engine.cpp 2016-07-06 18:40:24 +0000
3+++ organizer/qorganizer-eds-engine.cpp 2016-08-23 19:16:27 +0000
4@@ -2301,7 +2301,11 @@
5 switch (qRule.limitType()) {
6 case QOrganizerRecurrenceRule::DateLimit:
7 if (qRule.limitDate().isValid()) {
8- rule->until = icaltime_from_timet(QDateTime(qRule.limitDate()).toTime_t(), TRUE);
9+ rule->until = icaltime_null_time();
10+ rule->until.year = qRule.limitDate().year();
11+ rule->until.month = qRule.limitDate().month();
12+ rule->until.day = qRule.limitDate().day();
13+ rule->until.is_date = 1;
14 }
15 break;
16 case QOrganizerRecurrenceRule::CountLimit:

Subscribers

People subscribed via source and target branches