On Mon, Apr 05, 2010 at 07:18:42PM +0200, Matthias Bolte wrote: > 2010/4/5 Eric Blake <eblake@xxxxxxxxxx>: > > 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. > > > > DanPB's argument [1] against marking log messages for translation was > mainly that they aren't meant for the end user, but for debugging > purpose and to be included in bug reports. It would also put a huge burden on the translators since these log messages are not easily translated & there are a hell of alot of them > On the other hand one can argue about VIR_ERROR on the libvirtd side > where it's used to report critical errors like the one you quoted > above. I could probably be convinced about VIR_ERROR translation :-) 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list