On 5/5/23 06:15, James Seo wrote:
On Thu, May 04, 2023 at 08:29:57AM -0700, Guenter Roeck wrote:
Please please please no such changes. I don't want to have to deal with
patch wars just because people believe variables should have other names.
Such changes add zero value unless one counts wasted review time as a "value".
Guenter
Hello,
Of course arbitrarily changing variable names is pointless. But this
patch fulfills the intent of 848ba0a2f20dc121a3ef5272a24641d2bd963d8b,
which makes this change for devm_hwmon_device_register_with_info() in
hwmon-kernel-api.txt and in hwmon.h - but not in hwmon.c. The same
commit makes the same change for hwmon_device_register_with_info() in
all three files, so it obviously should have been in tree already.
The other reason for this patch is that for the purpose of generating
function documentation from kerneldocs, it is not feasible to call
this parameter "extra_groups" in the kerneldoc and still call it
"groups" in the function itself. Doing so results in the lines
"const struct attribute_group **groups / undescribed" and no mention
of "extra_groups" in the generated document. Leaving things as is, so
that [devm_]hwmon_device_register_with_info() have different names
for this parameter, is potentially confusing and more noticeable to
to the eye than I would like once rendered.
Is this good enough to proceed? And as a subsystem maintainer, is
there anything else, specifically or in general, that you would like
Marginally. That should have been explained in more detail in the
description.
to see addressed?
I don't know. There are changes which seem to be more based on POV
than real improvement (such as the removal of the credit from the
PMBus document). I'll have to verify each and every reference to determine
if the change is appropriate. Also, the changes are substantial.
It will take a lot of time for me to review, and right now I do not have
that time. I have a hard time keeping up with code reviews.
Guenter
