Rename imIAccountBuddy to prplIAccountBuddy

RESOLVED FIXED in 1.6

Status

Chat Core
General
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: clokep, Assigned: clokep)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
*** 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
(Assignee)

Comment 1

3 years ago
Created attachment 8524976 [details] [diff] [review]
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.
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #8524976 - Flags: review?(florian)
Attachment #8524976 - Flags: review?(aleth)
(Assignee)

Comment 2

3 years ago
Created attachment 8524977 [details] [diff] [review]
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.
Attachment #8524977 - Flags: review?(aleth)
(Assignee)

Updated

3 years ago
Blocks: 954049

Updated

3 years ago
Attachment #8524977 - Flags: review?(aleth) → review+

Comment 3

3 years ago
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)
(Assignee)

Comment 4

3 years ago
Created attachment 8528334 [details] [diff] [review]
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.
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 6

3 years ago
Comment on attachment 8528334 [details] [diff] [review]
Patch v3

Thanks for improving this!
Attachment #8528334 - Flags: review?(aleth) → review+

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 7

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

Comment 8

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

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6

Comment 9

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

Comment 10

3 years ago
(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!
(Assignee)

Comment 11

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

Committed the correct version: http://hg.mozilla.org/comm-central/rev/b04d7ef86701
You need to log in before you can comment on or make changes to this bug.