On Tuesday, September 14, 2021 11:24:05 AM CEST Dan Carpenter wrote:
> On Mon, Sep 13, 2021 at 08:09:59PM +0200, Fabio M. De Francesco wrote:
> > Clean up usbctrl_vendorreq () in usb_ops_linux.c because some
> > of its code will be reused in this series. This cleanup is in
> > preparation for shortening the call chains of rtw_read{8,16,32}()
> > and rtw_write{8,16,32,N}(). More insights about the reasons why at
> > https://lore.kernel.org/lkml/5319192.FrU0QrjFp7@localhost.localdomain/
> >
>
> This commit message is quite bad.
>
> This patch has nothing to do with reusing the code or shortening call
> chains.
It has to do, in a certain sense. Let me explain please...
Some days ago, David Laight made the review of "Shorten calls chain of
rtw_write8/16/32/n()" version 3.
In that patch he noticed some lines of usb_read() that I had created with the
help of reusing some lines of the code of usbctrl_vendorreq() that is deleted
in the same patch.
He thought that they were clean-ups and renames and so he suggested to make
those "clean-ups" in a separate patch.
However they were _not_ renames or other clean-ups, because usb_read() was
not touched in that patch and, above all, it was a new function.
I am sure that when I write new functions I can use whatever name of
variables I like, even if people may think I'm renaming the variables that
were in a old function that now is deleted. Am I not permitted?
However, because I also think that readability of the diffs matters, I
decided to do some clean-up of the code I'm about to reuse in the new
functions. It improves readability of the above-mentioned patch that is also
the 18/18 of this series.
That is the reason why I'm cleaning up a function that is going to be deleted
in the last patch of the series.
> Don't use a link like that in the commit message especially when it's a
> link to an email you wrote. If it's someone else's email you can say,
> something like "As <name> points out in <his/her> email <url>. Blah
> blah blah." That way you give credit to the other person but all the
> information is in the commit message.
I agree with you. I'll redo the commit message for in order to summarize in
few lines why I'm doing clean-ups of functions that must be deleted in 18/18.
The same for 16/18. I think that a short explanation like the one that I gave
you above should suffice (much shorter, obviously).
I hope that I've been clear now. Please let me know if you have more
suggestions about this patch and the next (16/18).
Regards,
Fabio
