On Tue, Mar 29, 2022 at 07:48:03PM -0500, Rebecca Mckeever wrote: > On Tue, Mar 29, 2022 at 09:16:32AM -0700, Alison Schofield wrote: > > On Tue, Mar 29, 2022 at 02:53:36AM -0500, Rebecca Mckeever wrote: > > > Align the if and else if branches of the conditional statement > > > to improve readability. Prevent bugs that could be introduced > > > if developers misread the code. Issue found by checkpatch. > > > > Thanks for the patch Rebecca! > > > > Lots of stuff done right - passes chkp, compiles, patch is sent to > > correct recipients, the commit message follows the format of the file. > > > > Let's set a pattern here for all checkpatch related cleanups, > > for you and others that follow.(Thanks for being the first ;)) > > > > Commit msg says 'what'. Commit log says 'why'. Acknowledge that > > it was found using checkpatch in the commit log also. (In the future > > you may be acknowledging use of other tools like sparse, coccinelle.) > > > > Note that the 'why' is never that a tool reported an error. The 'why' > > for these checkpatch reports is usually to follow the Linux Kernel > > Coding Style. > > > > 'Fix' in the commit message is needlessly generic. Perhaps: > > [PATCH] staging: r8188eu: align both branches of a conditional statement > > > > Commit log: (what you have is fine in the log) > > I usually paste in the checkpatch error explicitly so it can be grep'd > > for. Something like: > > > > Issue found by checkpatch: > > WARNING: suspect code indent for conditional statements > > There was a section of https://kernelnewbies.org/PatchPhilosophy that suggested > putting the warning message in the subject line. I thought it would be > redundant to also put it in the body. Is it a good practice to include the > warning message in both places? > The commit msg should say what you did. If it happens to match the warning message, that's fine. Think of the commit msg as directives to a machine. This patch has already been applied :) https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=20b4b3fb383b3a499b8b47daaf1d6325faa9cfe2 You can view contents of staging-testing here: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/log/?h=staging-testing > > > > Thanks, > > Alison > > > > > > > > Signed-off-by: Rebecca Mckeever <remckee0@xxxxxxxxx> > > > --- > > > drivers/staging/r8188eu/core/rtw_cmd.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c > > > index 6eca30124ee8..ccc43c0ba433 100644 > > > --- a/drivers/staging/r8188eu/core/rtw_cmd.c > > > +++ b/drivers/staging/r8188eu/core/rtw_cmd.c > > > @@ -1408,7 +1408,7 @@ void rtw_survey_cmd_callback(struct adapter *padapter, struct cmd_obj *pcmd) > > > /* TODO: cancel timer and do timeout handler directly... */ > > > /* need to make timeout handlerOS independent */ > > > _set_timer(&pmlmepriv->scan_to_timer, 1); > > > - } else if (pcmd->res != H2C_SUCCESS) { > > > + } else if (pcmd->res != H2C_SUCCESS) { > > > _set_timer(&pmlmepriv->scan_to_timer, 1); > > > } > > > > > > -- > > > 2.32.0 > > > > > > > > > > Thanks, > Rebecca
