Add Purple.Account:protocol

Review Request #3643 — Created Nov. 4, 2024 and submitted

Information

pidgin/pidgin
default

Reviewers

This is construct only property which allows us to do more complicated testing.
This also caches lookups based on Purple.Account:protocol-id.
  • Opened in a devenv and verified my accounts worked.
  • Started creating a new account and verified that changing the protocol didn't throw any warnings.
  • Modified an existing account by changing its protocol without issue.
  • Deprecated purple_account_get_protocol_name as there's no good reason for it.
  • Called in the turtles.
Summary ID
Add Purple.Account:protocol
This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.
5eb57eb5df7d1930694e1ca09108b32bb645ab66
Description From Last Updated

Since no callers changed, I'm a bit uncertain why both protocol and protocol-id need to be settable? Can we not …

QuLogicQuLogic

Should this be g_assert_no_error like the other time?

QuLogicQuLogic
grim
QuLogic
  1. 
      
  2. Show all issues

    Since no callers changed, I'm a bit uncertain why both protocol and protocol-id need to be settable? Can we not cache protocol on first use from protocol-id or vice-versa, or are there other changes planned?

    1. protocol is construct only so we can set it from a unit test with g_object_new(PURPLE_TYPE_ACCOUNT, "protocol", prpl1, NULL);.

      protocol-id can be written whenever which we need to be able to do to let users change the protocol in the account editor. However, we don't need a protocol in this case until purple_account_get_protocol is called and when that happens we look up the protocol in the ProtocolManager like we did before, but this time we cache it.

      In the unit tests, we don't have the protocol manager unless we initialize all of libpurple which means our tests will be slower, it's possible other protocols might interfer, and so on. So to avoid all that, being able to create an account with an instance of a test protocol like in this review request makes things much easier and faster to run.

    2. So there's no need for some corresponding test changes then, yet?

    3. We could add tests to check it now, but I made this explicitly to enable testing in /r/3644/

  3. 
      
grim
QuLogic
  1. 
      
  2. libpurple/tests/test_account.c (Diff revision 3)
     
     
    Show all issues

    Should this be g_assert_no_error like the other time?

  3. 
      
grim
QuLogic
  1. Ship It!
  2. 
      
grim
Review request changed
Status:
Completed