Re: [libvirt] [PATCH v3 05/14] virsh: Create macro for common "config" option
On 01/12/2016 06:40 AM, Pavel Hrdina wrote:
On Tue, Jan 12, 2016 at 10:53:03AM +0100, Andrea Bolognani wrote:
> On Tue, 2016-01-12 at 10:41 +0100, Pavel Hrdina wrote:
>>
>> Just a question, I know it's to late to change those patches, it's pushed
now,
>> but why don't we unify the help string for all the commands? It does the
same
>> thing for all commands, there is no reason to have different help string for
>> some commands. And I don't think, that it would break anything.
>
> The most commonly used help text is "affect next boot", while eg. the
> help text for the 'schedinfo' command is "get/set value to be used on
> next boot".
>
> In this case it makes sense to have a different help text, because
> the information can not only be set but also retrieved.
Yes, that's true and the "affect next boot" is confusing in this case.
>
> That said, if you can come up with a help text that can accurately
> describe all situations where the 'config' option is used, I would
> certainly not oppose it :)
What about "affect offline definition", for live "affect running
definition"
and for current "affect current definition"? What each command does is under
"DESCRIPTION" and there is no need to repeat that information for each option.
So now that there's only one place to change, adjusting the message is
probably going to be a bit easier... Changing the text of the message
seemed "out of context" for what I was trying to accomplish though. I
don't disagree some messages could use an update especially considering
how the code as evolved over time.
Going through and making each of the help strings meaningful may be an
interesting exercise... My favorite helpstr's found during this exercise
were 'N_("file")' and 'N_("XML file")' - those were so
(ahem) helpful.
With the current code in place a "grep N_\(\" tools/virsh-*.c | sort -i
| uniq -c | sort -n" will give you an interesting snapshot of
helpstrings. Adding an additional "| grep VIRSH_COMMON_OPT" will drill
into the ones that were duplicated numerous times, but now affect
multiple commands.
John