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

Re: [libvirt] [PATCH] [2/6] Add an error module and message for the hooks subsystem



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 :|


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