GNOME Bugzilla – Bug 726229
View events from different boots
Last modified: 2015-07-01 15:11:37 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.
Created attachment 306502 [details] [review] patch for this bug
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.
Created attachment 306529 [details] [review] patch updated
Review of attachment 306529 [details] [review]: Great work! I pushed to master as commit 45c6a49a84d730165d107f3677215912ffe18e79.