From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751219AbdBWI4e (ORCPT ); Thu, 23 Feb 2017 03:56:34 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:34947 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751098AbdBWI4c (ORCPT ); Thu, 23 Feb 2017 03:56:32 -0500 Subject: Re: [Outreachy kernel] Re: [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full To: Julia Lawall , Tahia Khan References: <20170222171403.GA20626@coolbox> <838d3e67-646d-b2d3-ef7d-5812675db6db@broadcom.com> <20170223035400.GA9378@coolbox> Cc: outreachy-kernel@googlegroups.com, aditya.shankar@microchip.com, ganesh.krishna@microchip.com, gregkh@linuxfoundation.org, linux-wireless@vger.kernel.org, devel@driverdev.osusl.org, linux-kernel@vger.kernel.org From: Arend Van Spriel Message-ID: <1ffdd756-aaa4-5e1e-b72f-3e5d1f2daeb2@broadcom.com> Date: Thu, 23 Feb 2017 09:56:11 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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