Refactor old getter and setter to dbus property in bus/ibusimpl.c

Refactor old getter and setter to dbus property in bus/ibusimpl.c

[fujiwara, 2013-05-17 15:27:17]

Separated CL from CL #7712047. Part II.

Sort properties in ibusimpl.c

[fujiwara, 2013-05-22 06:09:13]

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode167
bus/ibusimpl.c:167: "    <property name='PreloadEngines' type='(as)'
access='write' />\n"
Maybe tuple is not useful in ibus_bus_set_ibus_property.

[Peng, 2013-05-23 03:47:44]

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode159
bus/ibusimpl.c:159: *
http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-p...
*/
I remember org.freedesktop.Properties has PropertiesChanged signal. Can we add
it too?

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode198
bus/ibusimpl.c:198: "    <!-- Use
/desktop/ibus/general/use_system_keyboard_layout -->\n"
dont think it is necessary to add deprecated properties.

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode211
bus/ibusimpl.c:211: "-->\n"
dont think comments in string is necessary

https://codereview.appspot.com/9477043/diff/1/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/1/src/ibusbus.c#newcode576
src/ibusbus.c:576: static gchar *
const gchar*?

[fujiwara, 2013-05-28 15:27:08]

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode159
bus/ibusimpl.c:159: *
http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-p...
*/
On 2013/05/23 03:47:44, Peng wrote:
> I remember org.freedesktop.Properties has PropertiesChanged signal. Can we add
> it too?

I'm thinking to emit PropertiesChanged in bus_ibus_impl_set_focused_context()
but I don't know how to get GDBusConnection.

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode167
bus/ibusimpl.c:167: "    <property name='PreloadEngines' type='(as)'
access='write' />\n"
On 2013/05/22 06:09:13, fujiwara wrote:
> Maybe tuple is not useful in ibus_bus_set_ibus_property.

I removed tuple.

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode198
bus/ibusimpl.c:198: "    <!-- Use
/desktop/ibus/general/use_system_keyboard_layout -->\n"
On 2013/05/23 03:47:44, Peng wrote:
> dont think it is necessary to add deprecated properties. 

OK, I deleted UseSysLayout property.
Engines and ActiveEngines properties are shared codes in src/ibusbus.c

https://codereview.appspot.com/9477043/diff/1/bus/ibusimpl.c#newcode211
bus/ibusimpl.c:211: "-->\n"
On 2013/05/23 03:47:44, Peng wrote:
> dont think comments in string is necessary

Deleted the comments.

https://codereview.appspot.com/9477043/diff/1/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/1/src/ibusbus.c#newcode576
src/ibusbus.c:576: static gchar *
On 2013/05/23 03:47:44, Peng wrote:
> const gchar*?

I changed the introspection and now this works.

[Peng, 2013-05-29 14:31:00]

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c#newcode956
bus/ibusimpl.c:956: gchar *path = g_variant_dup_string (variant, NULL);
why not g_variant_get_string() to avoid dup & free?

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c#newcode1097
bus/ibusimpl.c:1097: g_variant_builder_init (&builder, G_VARIANT_TYPE ("av"));
I don't understand why not use variant returned from _ibus_get_engines()
directly?

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c#newcode1189
bus/ibusimpl.c:1189: g_variant_builder_init (&builder, G_VARIANT_TYPE ("av"));
same question?

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode742
src/ibusbus.h:742: * Deprecated API. Use
/desktop/ibus/general/use_system_keyboard_layout.
This comment is not clear for.
What does it mean 'Use /desktop/ibus/general/use_system_keyboard_layout'

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode746
src/ibusbus.h:746: (IBusBus        *bus);
I think gcc has a better way to mark a function deprecated

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode804
src/ibusbus.h:804: * Deprecated API. If we use local engines.
I don't understand this line!

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode823
src/ibusbus.h:823: * Deprecated API. If we use local engines.
same

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode836
src/ibusbus.h:836: * Deprecated API. This would be for chrome.
ibus_set_log_handler() is
same.

[fujiwara, 2013-05-30 15:38:14]

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c#newcode956
bus/ibusimpl.c:956: gchar *path = g_variant_dup_string (variant, NULL);
On 2013/05/29 14:31:00, Peng wrote:
> why not g_variant_get_string() to avoid dup & free?

OK, I changed it.

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c#newcode1097
bus/ibusimpl.c:1097: g_variant_builder_init (&builder, G_VARIANT_TYPE ("av"));
On 2013/05/29 14:31:00, Peng wrote:
> I don't understand why not use variant returned from _ibus_get_engines()
> directly?

Now I could use _ibus_get_engines() directly with your suggestion.

https://codereview.appspot.com/9477043/diff/7001/bus/ibusimpl.c#newcode1189
bus/ibusimpl.c:1189: g_variant_builder_init (&builder, G_VARIANT_TYPE ("av"));
On 2013/05/29 14:31:00, Peng wrote:
> same question?

Done.

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode742
src/ibusbus.h:742: * Deprecated API. Use
/desktop/ibus/general/use_system_keyboard_layout.
On 2013/05/29 14:31:00, Peng wrote:
> This comment is not clear for.
> What does it mean 'Use /desktop/ibus/general/use_system_keyboard_layout'

I updated comments.

https://codereview.appspot.com/9477043/diff/7001/src/ibusbus.h#newcode746
src/ibusbus.h:746: (IBusBus        *bus);
On 2013/05/29 14:31:00, Peng wrote:
> I think gcc has a better way to mark a function deprecated

OK, I added IBUS_DEPRECATED .

[Peng, 2013-05-30 23:56:21]

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c#newcode168
bus/ibusimpl.c:168: "      <annotation
name='org.freedesktop.DBus.Property.EmitsChangedSignal'\n"
>80

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c#newcode201
bus/ibusimpl.c:201: "<!-- Start of deprecated properties -->\n"
remove comment from string

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c#newcode204
bus/ibusimpl.c:204: "<!-- End of deprecated properties -->\n"
same

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1551
src/ibusbus.c:1551: G_VARIANT_TYPE ("(v)"));
is not (av)?

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1618
src/ibusbus.c:1618: g_variant_get (result, "(v)", &variant);
why the signature of ListEngine is changed?

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1790
src/ibusbus.c:1790: #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
why need this?

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h#newcode802
src/ibusbus.h:802: * This is deprecated unless local engines are used.
Remove This is deprecated .....

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h#newcode820
src/ibusbus.h:820: * This is deprecated unless local engines are used.
same

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h#newcode842
src/ibusbus.h:842: * This is deprecated unless local engines are used.
same

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h#newcode1116
src/ibusbus.h:1116: void         ibus_bus_get_ibus_property_async
It is better to have some test for those new functions.

https://codereview.appspot.com/9477043/diff/14001/src/ibusversion.h.in
File src/ibusversion.h.in (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusversion.h.in#newcode70
src/ibusversion.h.in:70: #define IBUS_DEPRECATED G_GNUC_DEPRECATED
I think it is better to use G_GNUC_DEPRECATED directly.

[fujiwara, 2013-05-31 03:13:15]

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c#newcode168
bus/ibusimpl.c:168: "      <annotation
name='org.freedesktop.DBus.Property.EmitsChangedSignal'\n"
On 2013/05/30 23:56:21, Peng wrote:
> >80 

Done.

https://codereview.appspot.com/9477043/diff/14001/bus/ibusimpl.c#newcode201
bus/ibusimpl.c:201: "<!-- Start of deprecated properties -->\n"
On 2013/05/30 23:56:21, Peng wrote:
> remove comment from string

Done.

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1551
src/ibusbus.c:1551: G_VARIANT_TYPE ("(v)"));
On 2013/05/30 23:56:21, Peng wrote:
> is not (av)?

org.freedesktop.DBus.Properties.Get method always has the return value of "(v)".

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1618
src/ibusbus.c:1618: g_variant_get (result, "(v)", &variant);
On 2013/05/30 23:56:21, Peng wrote:
> why the signature of ListEngine is changed?

It's not ListEngine only but also all signatures are "(v)" for the get method.

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1790
src/ibusbus.c:1790: #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
On 2013/05/30 23:56:21, Peng wrote:
> why need this?

GCC outputs the compile error of the deprecated function.
The g_simple_async_result_is_valit() refers the deprecated
ibus_bus_get_use_sys_layout_async().

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h#newcode802
src/ibusbus.h:802: * This is deprecated unless local engines are used.
On 2013/05/30 23:56:21, Peng wrote:
> Remove This is deprecated .....

Done.

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.h#newcode1116
src/ibusbus.h:1116: void         ibus_bus_get_ibus_property_async
On 2013/05/30 23:56:21, Peng wrote:
> It is better to have some test for those new functions.

OK, I updated src/tests/ibus-bus.c

https://codereview.appspot.com/9477043/diff/14001/src/ibusversion.h.in
File src/ibusversion.h.in (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusversion.h.in#newcode70
src/ibusversion.h.in:70: #define IBUS_DEPRECATED G_GNUC_DEPRECATED
On 2013/05/30 23:56:21, Peng wrote:
> I think it is better to use G_GNUC_DEPRECATED directly.

Right. I mistook it.

[Peng, 2013-06-03 15:04:35]

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1790
src/ibusbus.c:1790: #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
On 2013/05/31 03:13:15, fujiwara wrote:
> On 2013/05/30 23:56:21, Peng wrote:
> > why need this?
> 
> GCC outputs the compile error of the deprecated function.
> The g_simple_async_result_is_valit() refers the deprecated
> ibus_bus_get_use_sys_layout_async().
Could you please add inline comment about it?

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcode23
src/tests/ibus-bus.c:23: #if 0
Why if 0?

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:534: result = ibus_bus_get_ibus_property (bus,
"CurrentInputContext");
Does it cover all new properties?

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:541: GAsyncResult *res,
align arguments

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:634: 
rmpty line?

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:657: //start_list_active_engines_async,
why comment out them?

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:706: //g_test_add_func ("/ibus/list-active-engines",
test_list_active_engines);
whye comment out them?

[fujiwara, 2013-06-04 14:35:58]

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcode23
src/tests/ibus-bus.c:23: #if 0
On 2013/06/03 15:04:35, Peng wrote:
> Why if 0?

Because the deprecated APIs are used.

[Peng, 2013-06-04 14:40:48]

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcode23
src/tests/ibus-bus.c:23: #if 0
On 2013/06/04 14:35:58, fujiwara wrote:
> On 2013/06/03 15:04:35, Peng wrote:
> > Why if 0?
> 
> Because the deprecated APIs are used.

In that case, we have two choices.
1. Add compile flags for this test to ignore the warnings.
2. Just remove them instead of #if 0

[fujiwara, 2013-06-04 16:59:55]

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/14001/src/ibusbus.c#newcode1790
src/ibusbus.c:1790: #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
On 2013/06/03 15:04:35, Peng wrote:
> On 2013/05/31 03:13:15, fujiwara wrote:
> > On 2013/05/30 23:56:21, Peng wrote:
> > > why need this?
> > 
> > GCC outputs the compile error of the deprecated function.
> > The g_simple_async_result_is_valit() refers the deprecated
> > ibus_bus_get_use_sys_layout_async().
> Could you please add inline comment about it? 

Done.

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcode23
src/tests/ibus-bus.c:23: #if 0
On 2013/06/04 14:40:49, Peng wrote:
> On 2013/06/04 14:35:58, fujiwara wrote:
> > On 2013/06/03 15:04:35, Peng wrote:
> > > Why if 0?
> > 
> > Because the deprecated APIs are used.
> 
> In that case, we have two choices.
> 1. Add compile flags for this test to ignore the warnings.
> 2. Just remove them instead of #if 0

I don't think adding #pragma is good for other compilers.
Anyway I removed #if 0.

https://codereview.appspot.com/9477043/diff/26001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:534: result = ibus_bus_get_ibus_property (bus,
"CurrentInputContext");
On 2013/06/03 15:04:35, Peng wrote:
> Does it cover all new properties?

Done.

[Peng, 2013-06-04 17:34:33]

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.c#newcode1769
src/ibusbus.c:1769: // ibus_bus_get_use_sys_layout_async is deprecated.
It is better to align with g_return_if_fail

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.c#newcode1793
src/ibusbus.c:1793: // ibus_bus_get_use_sys_layout_async is deprecated.
same

https://codereview.appspot.com/9477043/diff/36001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/36001/src/tests/ibus-bus.c#newcode30
src/tests/ibus-bus.c:30: #pragma GCC diagnostic ignored
"-Wdeprecated-declarations"
Is it possible to add the flag in Makefile.am for this test only?

[fujiwara, 2013-06-06 13:47:45]

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.c#newcode1769
src/ibusbus.c:1769: // ibus_bus_get_use_sys_layout_async is deprecated.
On 2013/06/04 17:34:33, Peng wrote:
> It is better to align with g_return_if_fail

Removed the comment because #pragma is not used.

https://codereview.appspot.com/9477043/diff/36001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/36001/src/tests/ibus-bus.c#newcode30
src/tests/ibus-bus.c:30: #pragma GCC diagnostic ignored
"-Wdeprecated-declarations"
On 2013/06/04 17:34:33, Peng wrote:
> Is it possible to add the flag in Makefile.am for this test only?

I understood this means to override CFLAGS to disable warnings but the CFLAGS is
a gcc specific option and needs to check if CC is gcc.

Now defining IBUS_DEPRECATED is preferred for me and updated the patch.

[Peng, 2013-06-12 17:49:11]

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.h#newcode690
src/ibusbus.h:690: G_GNUC_DEPRECATED
G_DEPRECATED please?

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.c#newcode23
src/ibusbus.c:23: #define IBUS_DISABLE_REPRECATION_WARNINGS
DEPRECATION?
I think it is better to put it in CFLAGS -DIBUS_DISABLE_REPRECATION_WARNINGS in
Makefile.am

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.h#newcode927
src/ibusbus.h:927: #endif // end of ifndef IBUS_DISABLE_DEPRECATED
Could we use /* */ style comment

https://codereview.appspot.com/9477043/diff/44001/src/ibusshare.h
File src/ibusshare.h (right):

https://codereview.appspot.com/9477043/diff/44001/src/ibusshare.h#newcode41
src/ibusshare.h:41: #ifdef IBUS_DISABLE_REPRECATION_WARNINGS
DEPRECATION?

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c#newcode4
src/tests/ibus-bus.c:4: #define IBUS_DISABLE_REPRECATION_WARNINGS
I think it is better to put it in CFLAGS -DIBUS_DISABLE_REPRECATION_WARNINGS in
Makefile.am

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c#newcode38
src/tests/ibus-bus.c:38: #endif // end of ifndef IBUS_DISABLE_DEPRECATED
same

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:455: #endif // end of IBUS_DISABLE_DEPRECATED
/* */

[fujiwara, 2013-06-13 08:03:05]

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/36001/src/ibusbus.h#newcode690
src/ibusbus.h:690: G_GNUC_DEPRECATED
On 2013/06/12 17:49:12, Peng wrote:
> G_DEPRECATED please?

You referred the old revision.

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.c#newcode23
src/ibusbus.c:23: #define IBUS_DISABLE_REPRECATION_WARNINGS
On 2013/06/12 17:49:12, Peng wrote:
> DEPRECATION?
> I think it is better to put it in CFLAGS -DIBUS_DISABLE_REPRECATION_WARNINGS
in
> Makefile.am

Done.

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/9477043/diff/44001/src/ibusbus.h#newcode927
src/ibusbus.h:927: #endif // end of ifndef IBUS_DISABLE_DEPRECATED
On 2013/06/12 17:49:12, Peng wrote:
> Could we use /* */ style comment

Done.

https://codereview.appspot.com/9477043/diff/44001/src/ibusshare.h
File src/ibusshare.h (right):

https://codereview.appspot.com/9477043/diff/44001/src/ibusshare.h#newcode41
src/ibusshare.h:41: #ifdef IBUS_DISABLE_REPRECATION_WARNINGS
On 2013/06/12 17:49:12, Peng wrote:
> DEPRECATION?

Done.

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c#newcode4
src/tests/ibus-bus.c:4: #define IBUS_DISABLE_REPRECATION_WARNINGS
On 2013/06/12 17:49:12, Peng wrote:
> I think it is better to put it in CFLAGS -DIBUS_DISABLE_REPRECATION_WARNINGS
in
> Makefile.am

Done.

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c#newcode38
src/tests/ibus-bus.c:38: #endif // end of ifndef IBUS_DISABLE_DEPRECATED
On 2013/06/12 17:49:12, Peng wrote:
> same

Done.

https://codereview.appspot.com/9477043/diff/44001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:455: #endif // end of IBUS_DISABLE_DEPRECATED
On 2013/06/12 17:49:12, Peng wrote:
> /* */

Done.

[Peng, 2013-06-13 12:13:17]

lgtm with comments

https://codereview.appspot.com/9477043/diff/53001/src/tests/ibus-bus.c
File src/tests/ibus-bus.c (right):

https://codereview.appspot.com/9477043/diff/53001/src/tests/ibus-bus.c#newcod...
src/tests/ibus-bus.c:453: #endif /* end of IBUS_DISABLE_DEPRECATED */
I think only /* IBUS_DISABLE_DEPRECATED */ is enough. and for others