On Fri, Mar 26, 2010 at 05:42:22PM +0100, Daniel Veillard wrote:
On Fri, Mar 26, 2010 at 10:16:37AM -0600, Eric Blake wrote:
On 03/26/2010 09:42 AM, Daniel Veillard wrote:
@@ -1117,6 +1120,11 @@ virErrorMsg(virErrorNumber error, const char *info) errmsg = _("Failed to make domain persistent after migration"); else errmsg = _("Failed to make domain persistent after migration: %s"); + case VIR_ERR_HOOK_SCRIPT_FAILED: + if (info == NULL) + errmsg = _("Hook script execution failed"); + else + errmsg = _("Hook script execution failed: %s");
We don't have a very consistent style. A quick glance at virErrorMsg shows that maybe half of the messages start lower-case, and the other half start upper-case. GNU style recommends that error messages start with lower-case letters (unless the first word is an acronym, such as in the error _("GET operation failed") for VIR_ERR_GET_FAILED), and although this is not a GNU project, there is something to be said for consistency.
Is it worth a separate patch to make error message consistently start with lower-case? maint.mk even has a syntax-check rule that we can use to help in this regards (sc_error_message_uppercase), although we are not currently using it.
At any rate, capitalization can be a separate cleanup patch, so ACK to this one.
and in general that duplication of _("") strings is just nasty, we need to fix it, this requires translators to type everything twice it's nasty, awful, and need to be fixed. I though about this again when adding the entry, but I didn't want to mix issues.
I did a proof of concept converting both of the switch() blocks into a VIR_ENUM style lookup a while ago, but its bit-rotted. It was much nicer though. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|