On 23-2-2017 8:08, Julia Lawall wrote: >> 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
