On 04/05/2010 12:01 PM, Eric Blake wrote: > > And I just got called away; rest of the review later after I eat. > Resuming my review... > @@ -203,9 +204,8 @@ static int interfaceListInterfaces(virConnectPtr conn, char **const names, int n > const char *errmsg, *details; > int errcode = ncf_error(driver->netcf, &errmsg, &details); > interfaceReportError(netcf_to_vir_err(errcode), > - "%s (netcf: %s - %s)", > - _("failed to list host interfaces"), > - errmsg, details ? details : ""); > + _("failed to list host interfaces (netcf: %s - %s)"), > + errmsg, details ? details : ""); If details is empty, this results in "msg (netcf: msg - )" which looks a bit awkward. Perhaps this construct should be written as: "%s (netcf: %s%s%s)", errmsg, details ? " - " : "", details ? details : "" But that is a separate issue from your patch. > if (nparams != 1) { > - testError(VIR_ERR_INVALID_ARG, "nparams"); > + testError(VIR_ERR_INVALID_ARG, "%s", _("nparams")); > goto cleanup; > } > if (STRNEQ(params[0].field, "weight")) { > - testError(VIR_ERR_INVALID_ARG, "field"); > + testError(VIR_ERR_INVALID_ARG, "%s", _("field")); > goto cleanup; > } > if (params[0].type != VIR_DOMAIN_SCHED_FIELD_UINT) { > - testError(VIR_ERR_INVALID_ARG, "type"); > + testError(VIR_ERR_INVALID_ARG, "%s", _("type")); > goto cleanup; Translators tend to hate strings like this. Single words are hard to translate without any context. I'm not sure how to improve it, though. So, I guess that means ACK to the content of this patch, after addressing my nit in part 1 of the review about splitting the cfg.mk changes into two commits. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list