GNOME Bugzilla – Bug 657705
tweak unix signal watch API
Last modified: 2011-09-02 16:24:11 UTC
the unix signal watching source should adhere to the conventions followed by the other sources: xyz_source_new xyz_add xyz_add_full where xyz in this case would be 'g_unix_signal_watch' compare with 'g_child_watch' for example. We could also consider renaming it to 'g_signal_watch' to indicate that this is not a unix-specific thing (we have signals on Windows too) but I'm neutral on that point. Even though it's a bit verbose, "unix signal" helps you to understand that it's not a gobject signal.
Created attachment 195200 [details] [review] unix signal watch: make API match other sources Change the unix signal watch API to match other sources in both available functions, names of those functions and order of the parameters to the _full function.
Review of attachment 195200 [details] [review]: ::: glib/glib-unix.c @@ +242,3 @@ guint +g_unix_signal_watch_add_full (int priority, + int signum, Doing this will definitely be annoying for everyone since we're swapping the meaning of two integers. But I dunno, it's a one time transition I guess. We should probably be proactive and prepare patches for some modules. ::: glib/glib-unix.h @@ +69,1 @@ Why are we adding _watch here? I guess it does match _child_watch_source_new() but honestly I think that one's broken - having both "watch" AND "source" in the name.
The other possibility is g_unix_signal_source_new, g_unix_signal_add, g_unix_signal_add_full. I think I might actually prefer that, indeed.
(In reply to comment #3) > The other possibility is g_unix_signal_source_new, g_unix_signal_add, > g_unix_signal_add_full. I think I might actually prefer that, indeed. Yeah, I dislike the "watch" term personally and decided that all uses of it are legacy; "source" is the canonical term for "thing attached to main context".
Created attachment 195260 [details] [review] unix signal watch: make API match other sources Change the unix signal watch API to match other sources in both available functions, names of those functions and order of the parameters to the _full function.
Review of attachment 195260 [details] [review]: The -sections.txt and .symbols files aren't updated for the new API. Other than that, looks good to me.
Created attachment 195262 [details] [review] unix signal watch: make API match other sources Didn't stage all changes before the amend. Trying again.
thanks