Closed Bug 955353 Opened 6 years ago Closed 5 years ago

Rename imIAccountBuddy to prplIAccountBuddy

Categories

(Chat Core :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

*** 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
Attached patch Patch v2 (obsolete) — Splinter Review
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.
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #8524976 - Flags: review?(florian)
Attachment #8524976 - Flags: review?(aleth)
Attached patch Purple part v2Splinter Review
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.
Attachment #8524977 - Flags: review?(aleth)
Blocks: 954049
Attachment #8524977 - Flags: review?(aleth) → review+
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 ;)
Attachment #8524976 - Flags: review?(aleth)
Attached patch Patch v3Splinter Review
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.
Attachment #8524976 - Attachment is obsolete: true
Attachment #8524976 - Flags: review?(florian)
Attachment #8528334 - Flags: review?(florian)
Attachment #8528334 - Flags: review?(aleth)
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...).
Attachment #8528334 - Flags: review?(florian) → review+
Comment on attachment 8528334 [details] [diff] [review]
Patch v3

Thanks for improving this!
Attachment #8528334 - Flags: review?(aleth) → review+
Keywords: checkin-needed
Keywords: checkin-needed
I made a couple of minor improvements that aleth thought sounded good on IRC. I'll push this soon.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
(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!
You need to log in before you can comment on or make changes to this bug.