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 723926 - Reimplement GOA logging in terms of GLib logging
Reimplement GOA logging in terms of GLib logging
Status: RESOLVED FIXED
Product: gnome-online-accounts
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Online Accounts maintainer(s)
GNOME Online Accounts maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-02-08 21:20 UTC by Giovanni Campagna
Modified: 2014-02-10 10:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Reimplement GOA logging in terms of GLib logging (3.89 KB, patch)
2014-02-08 21:21 UTC, Giovanni Campagna
reviewed Details | Review
Replace GOA logging with GLib logging (97.03 KB, patch)
2014-02-09 18:00 UTC, Giovanni Campagna
reviewed Details | Review
Replace GOA logging with GLib logging (103.91 KB, patch)
2014-02-10 10:39 UTC, Debarshi Ray
committed Details | Review

Description Giovanni Campagna 2014-02-08 21:20:33 UTC
In addition to simplifying the code, this ensures that
G_MESSAGES_DEBUG is honored, which in turn shuts up the
logging of debug messages by default, and avoids filling
the journal of everyone who's running a recent dbus-daemon.
Comment 1 Giovanni Campagna 2014-02-08 21:21:04 UTC
Created attachment 268529 [details] [review]
Reimplement GOA logging in terms of GLib logging
Comment 2 Debarshi Ray 2014-02-09 08:26:26 UTC
Review of attachment 268529 [details] [review]:

::: src/goabackend/goalogging.c
@@ +197,3 @@
+         _color_get (_COLOR_FG_MAGENTA), _color_get (_COLOR_BOLD_ON), thread_str, _color_get (_COLOR_RESET),
+         message,
+         _color_get (_COLOR_FG_BLACK), location, function, _color_get (_COLOR_RESET));

Looks like g_log can not colour the output using terminal escape sequences? Maybe we should just kill goalogging.[ch] and directly use g_debug, g_warning, etc..
Comment 3 Giovanni Campagna 2014-02-09 18:00:18 UTC
Created attachment 268597 [details] [review]
Replace GOA logging with GLib logging

In addition to simplifying the code, this ensures that
G_MESSAGES_DEBUG is honored, which in turn shuts up the
logging of debug messages by default, and avoids filling
the journal of everyone who's running a recent dbus-daemon.
The following map was used when converting log levels:
 - debug -> debug
 - notice -> message
 - info -> info
 - warning -> warning
 - error -> critical (because g_error() aborts the application)

Ok, so this is the more invasive patch that replaces all
goa_log() calls with the equivalent glib calls. In addition
to color, we lose also function and line reference, but I don't
believe this is a huge problem.
Comment 4 Debarshi Ray 2014-02-10 10:36:32 UTC
Review of attachment 268597 [details] [review]:

Looks good to me, apart from a few alignment issues caused by s/goa/g/. I will fix them locally and commit.

::: doc/goa-sections.txt
@@ -1074,3 @@
-</SECTION>
-
-<SECTION>

Thanks for taking care of the documentation.
Comment 5 Debarshi Ray 2014-02-10 10:37:14 UTC
Review of attachment 268597 [details] [review]:

Looks good to me, apart from a few alignment issues caused by s/goa/g/. I will fix them locally and commit.

::: doc/goa-sections.txt
@@ -1074,3 @@
-</SECTION>
-
-<SECTION>

Thanks for taking care of the documentation.
Comment 6 Debarshi Ray 2014-02-10 10:39:26 UTC
Created attachment 268652 [details] [review]
Replace GOA logging with GLib logging