[Pidgin] #811: add nullprpl protocol plugin

Pidgin trac at pidgin.im
Mon May 28 04:02:36 EDT 2007


#811: add nullprpl protocol plugin
------------------------+---------------------------------------------------
  Reporter:  ryanb      |       Owner:  rlaager                                       
      Type:  patch      |      Status:  new                                           
  Priority:  minor      |   Milestone:                                                
 Component:  libpurple  |     Version:  2.0                                           
Resolution:             |    Keywords:  nullprpl, null, protocol, mockprpl, nullclient
   Pending:  0          |  
------------------------+---------------------------------------------------
Comment (by rlaager):

 I've done a quick read of the patch. It looks good, with just a few minor
 issues.

 {{{
         268       if (full)
         269         purple_notify_user_info_add_pair(info, _("Tooltip
 style"), _("full"));
         270       else
         271         purple_notify_user_info_add_pair(info, _("Tooltip
 style:"), _("short"));
 }}}

 I'm sure you meant the strings to be the same, but they're not. Thinking
 about the translators, is this really necessary? I understand the example
 you're trying to provide, but you do use full later on. I think we can
 probably remove it.

 You have at least one link to
 http://snarfed.org/space/pidgin+null+protocol+plugin, which doesn't exist.

 Also, we use stuff like this at the tops of files, rather than a specific
 copyright line with a name. I could just change this when I commit this,
 but I would prefer your okay first.

 {{{
  * purple
  *
  * Purple is the legal property of its developers, whose names are too
 numerous
  * to list here.  Please refer to the COPYRIGHT file distributed with this
  * source distribution.
 }}}

 Finally, you're using assert(). If anything, I think you would want
 g_assert(), as I don't know how portable assert() is. That said, we don't
 use assert() in general. g_return_if_fail(), et al. allow for the program
 to carry on, rather than aborting right away. Glib also allows for an
 environment variable to be set to force things to abort() right away, so
 you can get those benefits when debugging, if desired.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/811#comment:10>
Pidgin <http://pidgin.im>
Pidgin


More information about the Tracker mailing list