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