Indicator Date and Time

Merge lp://qastaging/~charlesk/indicator-datetime/lp-917236 into lp://qastaging/indicator-datetime/12.10

lp-917236
Merge into trunk.12.10

Proposed by Charles Kerr on 2012-09-20

Status:

Merged

Approved by:

Lars Karlitski on 2012-09-20

Approved revision:

192

Merged at revision:

185

Proposed branch:

lp://qastaging/~charlesk/indicator-datetime/lp-917236

Merge into:

lp://qastaging/indicator-datetime/12.10

Diff against target:

185 lines (+64/-23)

2 files modified

src/datetime-service.c (+59/-23)
src/indicator-datetime.c (+5/-0)

To merge this branch:

bzr merge lp://qastaging/~charlesk/indicator-datetime/lp-917236

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
jenkins (community)	continuous-integration		Approve on 2012-09-20
Lars Karlitski (community)		2012-09-20	Approve on 2012-09-20
Review via email: mp+125388@code.qastaging.launchpad.net

Commit message

When clock skew is detected, rebuild the date/time labels.

Description of the change

Add a timer to monitor for clock skew, and when it's detected, rebuild the datetime menuitems that need it.

Larsu, I'd also like a second opinion on whether or not the SystemIdleHintChanged callback is still needed, since the clock skew code should have the same results...

Revision history for this message

jenkins (martin-mrazik+qa) wrote on 2012-09-20:

PASSED: Continuous integration, rev:187
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/6/
Executed test runs:
SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./label=quantal/6/console

review: Approve (continuous-integration)

lp://qastaging/~charlesk/indicator-datetime/lp-917236 updated on 2012-09-20

188. By Charles Kerr on 2012-09-20: clocks don't skew very often, so let's reduce the frequency that we test for this.

Revision history for this message

jenkins (martin-mrazik+qa) wrote on 2012-09-20:

PASSED: Continuous integration, rev:188
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/7/
Executed test runs:
SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/./label=quantal/7/console

review: Approve (continuous-integration)

Revision history for this message

Lars Karlitski (larsu) wrote on 2012-09-20:

Why are SKEW_CHECK_INTERVAL_SEC and SKEW_DIFF_THRESHOLD_SEC enums instead of #defines or const ints? I prefer having those kinds of definitions at the top of the file.

If we remove the SystemIdleHintChanged callback, the clock in the menu will be updated after a maximum of SKEW_CHECK_INTERVAL_SEC seconds, right? I don't like that, even if it is only the clock in the menu.

Why are the timers running when the menu is not open?

review: Needs Information

Revision history for this message

Charles Kerr (charlesk) wrote on 2012-09-20:

> I prefer having those kinds of definitions at the top of the file.

Agreed, r189.

> Why are SKEW_CHECK_INTERVAL_SEC and SKEW_DIFF_THRESHOLD_SEC enums instead of #defines or const ints?

const ints aren't legal when you define threshold in terms of interval. enums in general safer than preprocessor tokens. But, in terms of our codebase, you're right #defines are way more common. r190.

> Why are the timers running when the menu is not open?

To ensure that the indicator's label gets updated via the UpdateTime signal.

lp://qastaging/~charlesk/indicator-datetime/lp-917236 updated on 2012-09-20

189. By Charles Kerr on 2012-09-20: move declaration of SKEW_CHECK_INTERVAL_SEC and SKEW_DIFF_THRESHOLD_SEC to the top of the file.
190. By Charles Kerr on 2012-09-20: for SKEW_DIFF_THRESHOLD_SEC and SKEW_CHECK_INTERVAL_SEC, use #define instead of enum