[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