Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On 12/17/2015 11:04 AM, Markus Armbruster wrote:

>>> + * Create an error and add additional explanation:
>>> + *     error_setg(&err, "invalid quark");
>>> + *     error_append_hint(&err, "Valid quarks are up, down, strange, "
>>> + *                       "charm, top, bottom\n");
>>
>> Do we want to allow/encourage a trailing dot in the hint?  I'm fine if
>> we don't - but once we document its usage, we should probably then be
>> consistent with what we document; qemu-option.c used a trailing dot,
>> qdev-monitor.c did not.
> 
> I'll fix this example.
> 
>>> + *
>>> + * Do *not* contract this to
>>> + *     error_setg(&err, "invalid quark\n"
>>> + *                "Valid quarks are up, down, strange, charm, top, bottom");

Of course, if you put a trailing dot above, you'd also want it here in
the 'don't contract' section.


>>> @@ -128,6 +138,7 @@ ErrorClass error_get_class(const Error *err);
>>>   * If @errp is anything else, *@errp must be NULL.
>>>   * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its
>>>   * human-readable error message is made from printf-style @fmt, ...
>>> + * The resulting message should not contain newlines.
>>
>> Should we also discourage trailing punctuation?
> 
> Yes.  How to best phrase it?

Maybe:

The resulting message should be a single phrase, with no newline or
trailing punctuation.

> 
>> Should we also mention no need for leading 'error: ' prefix?
> 
> Unlike the other anti-patterns, this one doesn't seem frequent in new
> code.

Fair enough.


>>>  /**
>>>   * Append a printf-style human-readable explanation to an existing error.
>>> - * May be called multiple times, and safe if @errp is NULL.
>>> + * @errp may be NULL, but neither &error_fatal nor &error_abort.
>>
>> As a native speaker, 'not' sounds better than 'neither' in that
>> sentence.  But I think both choices are grammatically correct.
> 
> You mean "not &error_abort or &error_abort"?

Yes, that works:

@errp may be NULL, but not &error_fatal or &error_abort.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]