Created attachment 195404 [details] [review]
Actually delete gnome-tz-monitor.[ch]

The previous commit only removed them from Makefile.am.

Created attachment 195405 [details] [review]
Add GnomeWallClock

Since clocks are a bit tricky, it's useful to centralize this class.
See https://bugzilla.gnome.org/show_bug.cgi?id=655129
See https://bugzilla.gnome.org/show_bug.cgi?id=657074

Created attachment 195406 [details] [review]
GnomeWallClock: Add support for Linux timerfd()

See https://bugzilla.gnome.org/show_bug.cgi?id=655129 for more background
information on timerfd.

Review of attachment 195404 [details] [review]:

Already fixed actually:
http://git.gnome.org/browse/gnome-desktop/commit/?id=03915ec950307e526cab7fcc7b1245a8715a5405

Review of attachment 195405 [details] [review]:

General comments:
 - gtk-doc comments please :-)
 - as discussed on IRC, I'd go with a "changed" signal that would pass the string as argument to the handler, instead of the notify:: on the clock property. This looks nicer and easier to use (to me, at least), but if others disagree and prefer the notify:: way, I won't fight over it.

::: libgnome-desktop/gnome-wall-clock.c
@@ +70,3 @@
+	g_object_unref (tz);
+	
+	g_signal_connect (self->priv->tz_monitor, "changed", G_CALLBACK(on_tz_changed), self);

Missing space between G_CALLBACK and (. This happens also a few lines below, , and for G_OBJECT_CLASS (in dispose & finalize).

@@ +169,3 @@
+		g_source_remove (self->priv->clock_update_id);
+  
+	source = g_timeout_source_new_seconds (1);

(Obviously, we want the other patch to avoid this 1s timeout when we don't show seconds)

@@ +175,3 @@
+	g_source_unref (source);
+
+	format_type = g_settings_get_string (self->priv->desktop_settings, "clock-format");

Shouldn't this be an enum instead of a string? (I didn't look at the gsettings-desktop-schemas patch)

@@ +218,3 @@
+                  const char *key,
+                  gpointer user_data)
+{

It'd be nice to only update the clock if the key is relevant to us (ie, the keys used in update_clock).

Review of attachment 195406 [details] [review]:

Assuming the gnome-datetime-source.c has been reviewed for glib (so skipping that), everything is good (except for the small detail below). Thanks!

::: configure.ac
@@ +165,3 @@
 
+AC_CACHE_CHECK(for timerfd_create(2) system call,
+    glib_cv_timerfd,AC_COMPILE_IFELSE([AC_LANG_PROGRAM([

Just rename glib_cv_timerfd to something not using the glib namespace.

(In reply to comment #5)
>
> (Obviously, we want the other patch to avoid this 1s timeout when we don't show
> seconds)

We need to wake up once a second to detect wall clock changes (via date&time panel or via resume from suspend).

> @@ +175,3 @@
> +    g_source_unref (source);
> +
> +    format_type = g_settings_get_string (self->priv->desktop_settings,
> "clock-format");
> 
> Shouldn't this be an enum instead of a string? (I didn't look at the
> gsettings-desktop-schemas patch)

Hmm...yes.  I guess gsettings is doing some enum->string translation if you request via _get_string().

Attachment 195405 [details] pushed as fb9ee91 - Add GnomeWallClock
Attachment 195406 [details] pushed as 5f65d7d - GnomeWallClock: Add support for Linux timerfd()