After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726229 - View events from different boots
View events from different boots
Status: RESOLVED FIXED
Product: gnome-logs
Classification: Other
Component: general
git master
Other All
: Normal enhancement
: ---
Assigned To: gnome-logs maintainer(s)
gnome-logs maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-03-13 11:19 UTC by David King
Modified: 2015-07-01 15:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for this bug (20.85 KB, patch)
2015-07-01 13:33 UTC, Jonathan Kang
needs-work Details | Review
patch updated (20.80 KB, patch)
2015-07-01 14:58 UTC, Jonathan Kang
committed Details | Review

Description David King 2014-03-13 11:19:03 UTC
Currently, only events from the current boot are shown, and this is not very useful for post-mortem log analysis. There should be a way to page in old events as necessary, select a different boot or both.
Comment 1 Jonathan Kang 2015-07-01 13:33:37 UTC
Created attachment 306502 [details] [review]
patch for this bug
Comment 2 David King 2015-07-01 14:03:57 UTC
Review of attachment 306502 [details] [review]:

Generally good, just a few minor comments.

::: src/gl-eventtoolbar.c
@@ +70,3 @@
+
+        /* For the condition that there are less then five boots in system */
+        if (i >= boot_ids->len)

You can move this up to the condition in the for loop.

@@ +76,3 @@
+
+        boot_id = &g_array_index (boot_ids,
+                                  GlJournalBootID, boot_ids->len - 1 - i);

Why not just start at boot_ids->len, and decrement down to 0?

::: src/gl-eventtoolbar.h
@@ +45,3 @@
 void gl_event_toolbar_set_mode (GlEventToolbar *toolbar,
                                 GlEventToolbarMode mode);
+void gl_event_toolbar_add_boot (GlEventToolbar *toolbar, GArray *boot_ids);

Probably gl_event_toolbar_add_boots(), as this accepts an array.

::: src/gl-journal.c
@@ +46,3 @@
+static const gchar DESKTOP_SCHEMA[] = "org.gnome.desktop.interface";
+static const gchar CLOCK_FORMAT[] = "clock-format";
+static char match[42] = "_BOOT_ID=";

You need a comment here about why the array is 42 elements in size.

@@ +157,3 @@
+        now = g_date_time_new_now_local ();
+        boot_id.time = gl_util_timestamp_to_display (boot_id.realtime,
+                                                     now, clock_format);

Does it make sense to do the timestamp conversion here, or is it better in the UI? It seems that doing it in the UI would avoid having to do the GSettings schema lookup in this low-level journal reading code.
Comment 3 Jonathan Kang 2015-07-01 14:58:10 UTC
Created attachment 306529 [details] [review]
patch updated
Comment 4 David King 2015-07-01 15:11:25 UTC
Review of attachment 306529 [details] [review]:

Great work! I pushed to master as commit 45c6a49a84d730165d107f3677215912ffe18e79.