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.