Linux wireless drivers development
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: Tahia Khan <tahia.khan@gmail.com>
Cc: arend.vanspriel@broadcom.com, 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
Subject: Re: [Outreachy kernel] Re: [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
Date: Thu, 23 Feb 2017 08:08:14 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.20.1702230804410.2203@hadrien> (raw)
In-Reply-To: <20170223035400.GA9378@coolbox>

> 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.

julia

  reply	other threads:[~2017-02-23  7:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 17:14 [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full Tahia Khan
2017-02-22 19:50 ` Arend Van Spriel
2017-02-23  3:54   ` Tahia Khan
2017-02-23  7:08     ` Julia Lawall [this message]
2017-02-23  8:56       ` [Outreachy kernel] " Arend Van Spriel
2017-02-23 13:38         ` Julia Lawall
2017-02-23  6:24   ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1702230804410.2203@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=aditya.shankar@microchip.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=devel@driverdev.osusl.org \
    --cc=ganesh.krishna@microchip.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=outreachy-kernel@googlegroups.com \
    --cc=tahia.khan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox