[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 01/30] Don't mark log messages for translation



On 04/04/2010 11:36 AM, Matthias Bolte wrote:
> Also remove manually added function names from log messages, the logging
> macros already record them and the logging framework outputs them.

I see in [1] where the translation was first added, but no justification
there for why log messages are marked, nor in your patch why they should
not be marked.  I'm not opposed to the idea, but I think it would make a
lot more sense if the commit message explained exactly why we don't
think log messages deserve translations.

[1] https://www.redhat.com/archives/libvir-list/2009-January/msg00005.html

> +++ b/cfg.mk
> @@ -168,28 +168,16 @@ sc_prohibit_gethostby:
>  # |grep -vE '^(qsort|if|close|assert|fputc|free|N_|vir.*GetName|.*Unlock|virNodeListDevices|virHashRemoveEntry|freeaddrinfo|.*[fF]ree|xdrmem_create|xmlXPathFreeObject|virUUIDFormat|openvzSetProgramSentinal|polkit_action_unref)$'
>  
>  msg_gen_function =
> -msg_gen_function += DEBUG0

For example, I can easily see why DEBUG0 does not need translation (it
is for developers, and should never be expanded into a log message in a
distro's installation, so why waste the translator's efforts on it)...

> -msg_gen_function += DISABLE_fprintf
> -msg_gen_function += ERROR
> -msg_gen_function += ERROR0
> -msg_gen_function += REMOTE_DEBUG
>  msg_gen_function += ReportError
> -msg_gen_function += VIR_FREE
> -msg_gen_function += VIR_INFO

But the name VIR_INFO seems like it might be something the end user may
eventually want to see in their own language.  Maybe the reasoning that
I'm not seeing here is that grep-ability of system logs is more
important than translating system logs into the user's language?

> @@ -236,7 +223,7 @@ func_re := ($(func_or))
>  #    "%s", _("no storage vol w..."
>  sc_libvirt_unmarked_diagnostics:
>  	@grep -nE							\
> -	    '\<$(func_re) \([^"]*"[^"]*[a-z]{3}' $$($(VC_LIST_EXCEPT))	\
> +	    '\<$(func_re) *\([^"]*"[^"]*[a-z]{3}' $$($(VC_LIST_EXCEPT))	\
>  	  | grep -v '_''(' &&						\
>  	  { echo '$(ME): found unmarked diagnostic(s)' 1>&2;		\
>  	    exit 1; } || :

ACK to this hunk, independently of the overall issue on why we need this
patch.

> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 208ffca..68fe95a 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -272,7 +272,7 @@ remoteCheckCertFile(const char *type, const char *file)
>      struct stat sb;
>      if (stat(file, &sb) < 0) {
>          char ebuf[1024];
> -        VIR_ERROR(_("Cannot access %s '%s': %s"),
> +        VIR_ERROR("Cannot access %s '%s': %s",
>                    type, file, virStrerror(errno, ebuf, sizeof ebuf));
>          return -1;
>      }

Again, this seems like something the user would want to see in their own
language.  So while the rest of this patch looks mechanically correct,
I'm hesitant to give an ACK without more reasoning on which functions
deserve to bypass translation; it may requiring reducing the scope of
this patch.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]