Created attachment 305077 [details] [review]
patch for this bug

Here is the patch.

Review of attachment 305077 [details] [review]:

The commit message looks a bit long, as do some of the code lines, so double-check that things are wrapped to 79 characters.

Other than the specific comments below, this looks good!

::: src/gl-journal.c
@@ +324,3 @@
+    if (error != NULL)
+    {
+        g_debug ("%s", error->message);

You should give a string before the error here, something like "Errowhile getting transport from journal: ".

@@ +332,3 @@
+    if (error != NULL)
+    {
+        g_debug ("%s", error->message);

Similar comment about the debug message as above.

@@ +594,3 @@
 }
 
+

Stray extra line.

@@ +606,3 @@
+gl_journal_entry_get_uid (GlJournalEntry *entry)
+{
+  g_return_val_if_fail (GL_IS_JOURNAL_ENTRY (entry), 0);

Does it make sense to also return -1 here?

"
@@ +606,3 @@
+gl_journal_entry_get_uid (GlJournalEntry *entry)
+{
+  g_return_val_if_fail (GL_IS_JOURNAL_ENTRY (entry), 0);

Does it make sense to also return -1 here?
"

Comment on this: -1 represents invalid uid. A integer value should be returned in this function, I think returning -1 makes sense if we check whether uid is valid or not when we need use uid.

(In reply to Jonathan Kang from comment #3)
> Comment on this: -1 represents invalid uid. A integer value should be
> returned in this function, I think returning -1 makes sense if we check
> whether uid is valid or not when we need use uid.

It mostly depends on whether you consider that calling this function with an invalid GlJournalEntry should give an invalid UID as a response (-1) or simply return with 0. 0 has the downside that some log messages have a UID of 0, so the response may be confused with a valid UID. -1 has the advantage that it represents an invalid UID, but the downside that it can also represent a failure to obtain the UID (although this is extremely unlikely).

(In reply to David King from comment #4)
> It mostly depends on whether you consider that calling this function with an
> invalid GlJournalEntry should give an invalid UID as a response (-1) or
> simply return with 0. 0 has the downside that some log messages have a UID
> of 0, so the response may be confused with a valid UID. -1 has the advantage
> that it represents an invalid UID, but the downside that it can also
> represent a failure to obtain the UID (although this is extremely unlikely).

So What about we use another minus integer to represent the invalid uids, like -2. Is't fine?

-1 should be fine for the moment.

Created attachment 305148 [details] [review]
patch updated

Review of attachment 305148 [details] [review]:

Pushed, with a few changes. You did not update the line wrapping in the commit message, and did not change the g_return_val_if_fail() call to return -1. I also added gl-eventviewrow.c to po/POTFILE.in.