Create a manager for conversations.

Review Request #677 — Created May 28, 2021 and submitted

grim
pidgin/pidgin
default
391d7cd30c38
pidgin

I Skipped unit tests because need a connection and full protocol implementation to do it properly.

Compiled and ran with an IRC account.

  • 0
  • 0
  • 19
  • 0
  • 19
Description From Last Updated
grim
  1. 
      
  2. Decided to do some more work here, so don't check this out yet...

  3. 
      
grim
QuLogic
  1. 
      
  2. finch/gntconv.c (Diff revision 2)
     
     

    Do you really need two vars? Seems like g_list_delete_link would work.

  3. finch/gntconv.c (Diff revision 2)
     
     

    Should be l->data, I believe.

  4. finch/gntconv.c (Diff revision 2)
     
     

    Actually, now it seems like purple_conversations_get_all is called multiple times; can you not free here and start from list again later? I assume the conversation count doesn't change.

  5. finch/gntconv.c (Diff revision 2)
     
     

    Extra blank line.

  6. finch/gntconv.c (Diff revision 2)
     
     

    Same comments as the previous function.

  7. libpurple/purpleconversationmanager.h (Diff revision 2)
     
     

    #PurpleConversation\s or something like that?

  8. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     
     
     

    All callers have already checked these things, so they're redundant, except for purple_conversation_manager_find_chat_by_id, which passes name=NULL and is broken by this name != NULL check.

    1. You can drop the others too? It's not public.

    2. I always get hesitent with this as it could eventually get made public, but then again since it is named _internal that's probably fine.

  9. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     

    !func(conversation, userdata)

  10. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     

    The hash table should be freed here?

  11. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     

    Don't need the if.

  12. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     

    Funny wrap indent.

  13. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     

    Funny wrap indent.

  14. libpurple/purpleconversationmanager.c (Diff revision 2)
     
     

    Why is this guint instead of gint?

    1. i was going with guint because id's aren't typically negative, but I'll change it back.

  15. pidgin/gtkblist.c (Diff revision 2)
     
     

    Is gtkblist->menutrayicon not just created in the below deleted code, and thus unused now?

  16. pidgin/gtkconv.c (Diff revision 2)
     
     

    Same questions as in finch.

  17. 
      
grim
grim
QuLogic
  1. Ship It!
  2. 
      
grim
Review request changed

Status: Closed (submitted)

Loading...