Re: [Outreachy kernel] Re: [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full

On 23-2-2017 8:08, Julia Lawall wrote:

quoted

Thanks for the feedback Arend, I really appreciate it. I've decided to go with
these changes in my follow-up patch request:

- rename tstrRSSI to 'rssi_history_buffer' as Aren suggested since it makes the
purpose of the struct clear
- remove Hungarian notation from all tstrRSSI members' names
- change type of u8Full to bool since it's only ever 1 or 0
- change name of as8RSSI to 'samples' since this buffer is only ever used to
compute an average, and the "rssi" prefix is implied by the struct's name
- rename str_rssi to rssi_history in the network_info struct for clarity

Since my reasoning for these changes deviates from just "renaming to
avoid camel casing" (as in the original checkpatch.pl warning), would it still
make sense to submit all this in a single patch? I know my commit message
needs to change but I wonder if this is too much detail.

I would strongly suggest not to do it all in a single patch.  Even if these
changes are not very complicated conceptually, there is always a chance of
doing things wrong.  Taking the problems one by one will improve the chance
that the result is correct.  Also, the results will be easier for you and
others to review if each patch only does one thing.  And easier to revert
if needed later if something goes wrong.

It is all related to cleaning up stuff in a single struct which I
consider "one thing" here. To me it looks a bit silly if you rename one
struct member when it is obvious that the other two need to be renamed
as well. The only somewhat sensible split I see here is: 1) rename the
struct itself, 2) rename the struct members, and 3) rename str_rssi
member in struct network_info.

Regards,
Arend