Re: [Telepathy] rework of the UI

First of all thanks for comments !

> * Please try to avoid whitespace-only changes
yes but they should be fixed... I fix them when I see a wrong
indentation and it's not easy to make a different patch for each
white-space bug...

> * Use g_object_set() instead of setting up a gvalue and using
> g_object_set_property (and when using g_object_set, don't cast the object).
Ok sorry, the second time I forgotten that g_object_set() can do the
work easier :-(

> * g_value_transform looks suspicious, is that right? don't you just want
> to copy the value?
g_value_transform copy the value and make some casting too. For example
it accept to copy a g_value containing a G_TYPE_UINT to G_TYPE_INT. I
think g_value_copy will make a warning if types are not exactly the
same.

> * What's the reason for having the function that returns a hash table
> with all params?
Telepathy needs to have a GHashTable with all params of the account to
connect it. And gossip_account_param_get_all() can take a MASK to say "I
want only required params" for example. It will be very useful in the UI
when using telepathy because each protocol can have different
parameters, so the idea I had for the UI when creating a new account is
to make a nice GtkAssistant for each known protocol and a default page
for unknown protocols that displays only required parameters with a
"optional" tab to display all not necessary options that the user will
maybe want to tweak.


> * The quark args should not be called param_name, since they are not
> names, but quarks, however...
renamed param_id :-)

> * ...what's the reason really to have a datalist, when a hashtable would
> work just as fine and probably be simpler and less code?
Seems almost the same, Eitan Isaacson told me he would like to use
g_datalist. As I understand g_datalist can be more efficient because it
searches values using a GQuark (integer value) instead of a string...
but that's almost a matter of s/g_datalist_get_data/g_hash_table_lookup/

OK so I opened bug #355797 [1] for the GossipAccount rework. I'll post
there an updated patch for HEAD taking care of your comments.

Xavier Claessens.

[1] http://bugzilla.gnome.org/show_bug.cgi?id=355797

_______________________________________________
Gossip-dev mailing list
[email protected]
http://lists.imendio.com/mailman/listinfo/gossip-dev

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)

iD8DBQBFCDPw6dEBUn2qPAMRArafAJ0cii22KxaNEnM61UnkgDhlfb5PNwCbBnQ9
M04FqeNPKIj6tdrsa2qSoHk=
=0eKi
-----END PGP SIGNATURE-----