955353 - Rename imIAccountBuddy to prplIAccountBuddy

Status: RESOLVED FIXED

(Chat Core :: General, defect)

Description

[Patrick Cloke [:clokep]]

*** Original post on bio 1916 at 2013-04-02 13:19:00 UTC ***

Apparently imIAccountBuddy is supposed to actually be prplIAccountBuddy, something for protocols to implement.

I don't think this would affect any add-ons, but it might. (If they are, we most likely "own" them.)

See http://log.bezut.info/instantbird/130325/#m244

Comment 1

[Patrick Cloke [:clokep]]

Attachment: Patch v2

I'm not going to individually reply to the comments from bug 954049. I took the thoughts in them and used them as guidance for reworking all the sets of comments.

Comment 2

[Patrick Cloke [:clokep]]

Attachment: Purple part v2

This is actually not identical to attachment 8522963 [details] [diff] [review], an additional change was made to change NS_DECL_IMIACCOUNTBUDDY to NS_DECL_PRPLIACCOUNTBUDDY.

Comment 3

[aleth [:aleth]]

Comment on attachment 8524976 [details] [diff] [review]
Patch v2

Review of attachment 8524976 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/components/public/imIContactsService.idl
@@ +61,5 @@
> + * Remember that an imIContact can have multiple buddies (imIBuddy instances),
> + * each imIBuddy can have multiple account-buddies (prplIAccountBuddy instances)
> + * referencing it. To be explicit, the difference is that an imIBuddy represents
> + * a contact's account on a network, while a prplIAccountBuddy represents your
> + * connection to that account.

I understand the last sentence, but the reference to "account" is confusing. Is this clearer?

"To be explicit, the difference is that an imIBuddy represents all the contact's various accounts on a network, while a prplIAccountBuddy represents these in your own accounts on that network."

@@ +165,5 @@
> + *
> + * E.g. Our contact Alice has two accounts on the Foo network: @lic4 and
> + * alice88; and she has a single account on the Bar network: _alice_. This would
> + * result in an imIBuddy instance for each of these: @lic4, alice88, and _alice_
> + * that would all exist as part of the same imIContact.

I don't think that's right: you'd end up with two imIBuddies and three prplIAccountBuddies.

But maybe the misunderstanding is mine ;)

Comment 4

[Patrick Cloke [:clokep]]

Attachment: Patch v3

This takes into account the feedback from aleth and also revs the IDs. This also removes the prplIAccountBuddy reference from that ibIConvStatsService IDL since it was unused.

Comment 5

[Florian Quèze [:florian]]

Comment on attachment 8528334 [details] [diff] [review]
Patch v3

Review of attachment 8528334 [details] [diff] [review]:
-----------------------------------------------------------------

Congratulations for making this significantly clearer than it has ever been :-).

::: chat/components/public/imIContactsService.idl
@@ +166,5 @@
> + * E.g. Our contact Alice has two accounts on the Foo network: @lic4 and
> + * alice88; and she has a single account on the Bar network: _alice_. This would
> + * result in an imIBuddy instance for each of these: @lic4, alice88, and _alice_
> + * that would all exist as part of the same imIContact.
> + *

Nit: no need for the trailing empty line here.

@@ +226,5 @@
>  
> +/*
> + * A prplIAccountBuddy represents the connection on a network between one of the
> + * current user's accounts and a persons's account. E.g. if we're logged into
> + * the Foo network as BobbyBoy91 and want to talk to Alice, there are two

"there are" or "there may be"?

@@ +228,5 @@
> + * A prplIAccountBuddy represents the connection on a network between one of the
> + * current user's accounts and a persons's account. E.g. if we're logged into
> + * the Foo network as BobbyBoy91 and want to talk to Alice, there are two
> + * prplIAccountBuddy instances: @lic4 as seen by BobbyBoy91 or alice88 as seen
> + * by BobbyBoy91. Additionally, if we also login as 8ob, there would be @lic4 as

"there would be" or "there could be"?

I guess the reason why this is slightly ambiguous is that the comment doesn't say that all the possible account buddies are actually in the blist. The account buddies exist only if the user added all of them (or if the user used Trillian in the past...).

Comment 6

[aleth [:aleth]]

Comment on attachment 8528334 [details] [diff] [review]
Patch v3

Thanks for improving this!

Comment 7

[Patrick Cloke [:clokep]]

I made a couple of minor improvements that aleth thought sounded good on IRC. I'll push this soon.

Comment 8

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/cf49ca9bc214

Comment 9

[aleth [:aleth]]

http://hg.mozilla.org/users/florian_queze.net/purple/rev/0d07da8d0178

Comment 10

[aleth [:aleth]]

(In reply to Patrick Cloke [:clokep] from comment #7)
> I made a couple of minor improvements that aleth thought sounded good on
> IRC. I'll push this soon.

Bah, I missed this comment. Sorry!

Comment 11

[Patrick Cloke [:clokep]]

Backed out: https://hg.mozilla.org/comm-central/rev/f7d72a7b3cd6

Committed the correct version: http://hg.mozilla.org/comm-central/rev/b04d7ef86701