Re: [Qemu-devel] [PATCH REBASE/RESEND 1/4] qdev: Add a description field for qdev properties for documentation

[Markus Armbruster]

Anthony Liguori <address@hidden> writes:

> On 02/17/2011 07:06 AM, Amit Shah wrote:
>> On (Tue) 15 Feb 2011 [10:43:42], Anthony Liguori wrote:
>>
>>    
>>>>   #define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
>>>> -    DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
>>>> +    DEFINE_PROP_DRIVE("drive", _state, _conf.bs, ""),                   \
>>>>       DEFINE_PROP_UINT16("logical_block_size", _state,                    \
>>>> -                       _conf.logical_block_size, 512),                  \
>>>> +                       _conf.logical_block_size, 512, ""),              \
>>>>       DEFINE_PROP_UINT16("physical_block_size", _state,                   \
>>>> -                       _conf.physical_block_size, 512),                 \
>>>> -    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
>>>> -    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
>>>> -    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
>>>> +                       _conf.physical_block_size, 512, ""),             \
>>>> +    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0, ""), \
>>>> +    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0, ""), \
>>>> +        DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1, ""), \
>>>>       DEFINE_PROP_UINT32("discard_granularity", _state, \
>>>> -                       _conf.discard_granularity, 0)
>>>> +                       _conf.discard_granularity, 0, "")
>>>>        
>>> This is pretty horribly ugly.  If we were going this, we should at
>>> least introduce new defines that include a documentation field and
>>> not just add empty documentation fields.
>>>      
>> We've discussed this in the past.  Once this patch series gets in,
>> I'll work to fill in the documentation here along with the
>> maintainers.
>>    
>
> It means you're touching everything twice instead of touching
> everything once.  That's unnecessary churn and blame breakage.
>
> It's still just as greppable if you use a new name.

What names would you suggest?  DEFINE_PROP_<FOO>_WITH_DOCS()?  For all
fifteen <FOO>s?  Not a good idea, because the longer name makes doing
the right thing even less attractive.  We better encourage doing the
right thing.