Issue 4902051: Use GVariant as attachment for IBusSerializable.

Issue 4902051: Use GVariant as attachment for IBusSerializable. (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
13 years, 4 months ago by Peng

Modified:
13 years, 4 months ago

Reviewers:
Yusuke Sato, horo1

Base URL:
git@github.com:ibus/ibus.git@master

Visibility:
Public.

Description

Use GVariant as attachment for IBusSerializable. BUG=None TEST=Linux desktop

Patch Set 2 : Fix review issues #

Total comments: 19

Patch Set 3 : Fix review issues #

Created: 13 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+98 lines, -206 lines)			Patch
M	src/ibusserializable.h	View	1 2	2 chunks	+40 lines, -41 lines	0 comments	Download
M	src/ibusserializable.c	View		5 chunks	+26 lines, -150 lines	0 comments	Download
M	src/tests/ibus-serializable.c	View	1 2	2 chunks	+32 lines, -15 lines	0 comments	Download

Messages

Total messages: 6

Expand All Messages | Collapse All Messages

horo1

Thank you for the cl! http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c File src/tests/ibus-serializable.c (right): http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c#newcode151 src/tests/ibus-serializable.c:151: IBusSerializable *object = (IBusSerializable ...

13 years, 4 months ago (2011-08-19 01:14:29 UTC) #2

Peng

http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c File src/tests/ibus-serializable.c (right): http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c#newcode151 src/tests/ibus-serializable.c:151: IBusSerializable *object = (IBusSerializable *) ibus_serializable_deserialize (variant); On 2011/08/19 ...

13 years, 4 months ago (2011-08-19 01:47:49 UTC) #3

Yusuke Sato

http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c File src/tests/ibus-serializable.c (right): http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c#newcode134 src/tests/ibus-serializable.c:134: IBusText *text = ibus_text_new_from_string ("main text"); nit: _from_static_string would ...

13 years, 4 months ago (2011-08-19 03:54:18 UTC) #4

Peng

http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c File src/tests/ibus-serializable.c (right): http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c#newcode134 src/tests/ibus-serializable.c:134: IBusText *text = ibus_text_new_from_string ("main text"); On 2011/08/19 03:54:19, ...

13 years, 4 months ago (2011-08-19 04:34:47 UTC) #5

Yusuke Sato

13 years, 4 months ago (2011-08-19 06:11:10 UTC) #6

lgtm, thx

On 2011/08/19 04:34:47, Peng wrote:
> http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c
> File src/tests/ibus-serializable.c (right):
> 
>
http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c#ne...
> src/tests/ibus-serializable.c:134: IBusText *text =  ibus_text_new_from_string
> ("main text");
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > nit: _from_static_string would be better.
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/1/src/tests/ibus-serializable.c#ne...
> src/tests/ibus-serializable.c:179: g_variant_type_info_assert_no_infos ();
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > don't we have to unref the text object here?
> 
> Yes. Otherwise g_variant_type_info_assert_no_infos() will gives some errors.
> Because some resource used by GVariant are not released.
> 
> http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h
> File src/ibusserializable.h (right):
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:130: * @iter: An IBusMessageIter.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > should be @builder
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:144: * @iter: An IBusMessageIter.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > @variant
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:210: * @object: An IBusSerializable.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > ditto
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:222: * @object: An IBusSerializable.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > ditto
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:234: * @object: An IBusSerializable.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > ditto
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:246: * @object: An IBusSerializable.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > ditto
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:247: * @iter: An IBusMessageIter.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > remove this line
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:248: * @returns: TRUE if succeed; FALSE otherwise.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > this line seems to be incorrect. pls fix.
> 
> Done.
> 
>
http://codereview.appspot.com/4902051/diff/4001/src/ibusserializable.h#newcod...
> src/ibusserializable.h:259: * @iter: An IBusMessageIter.
> On 2011/08/19 03:54:19, Yusuke Sato wrote:
> > ditto
> 
> Done.

Expand All Messages | Collapse All Messages