[v8,0/6] staging: vt6655: a series of checkpatch fixes on the file: rxtx.c

  The fixes are similar, mostly long lines splitting. I had to make
serveral patches to ease the work of inspectors

v2: fixed a compilation error found by the kernel test robot and
recompiled the code

v3: tends out the error persisted in the second version. this version is
a correction of that

v4: did some corrections as recommended by Greg KH

v5: shortend changelog comments as recommended by Greg KH

v6: did some corrections as recommended by Greg KH

v7: fixed some errors on my changelog comments

v8: fixed some errors pointed out by Philipp Hortmann

Tanjuate Brunostar (6):
  staging: vt6655: fix lines ending in a '('
  staging: vt6655: fix long lines of code in s_uGetRTSCTSDuration
  staging: vt6655: fix long lines of code in s_uFillDataHead
  staging: vt6655: fix long lines of code in s_vGenerateTxParamete
  staging: vt6655: fix long lines of code in the rest of the file
  staging: vt6655: fix lines of code ending in a '('

 drivers/staging/vt6655/rxtx.c | 542 ++++++++++++++++++++--------------
 1 file changed, 324 insertions(+), 218 deletions(-)
  

We know it's a patchset or series, saying so in subject line is 
redundant. Perhaps - 'Checkpatch cleanup in rxtx.c'

On Fri, Oct 28, 2022 at 05:40:52PM +0000, Tanjuate Brunostar wrote:
> The fixes are similar, mostly long lines splitting. I had to make
> serveral patches to ease the work of inspectors
  ^
Please use a spell checker.

> 
> v2: fixed a compilation error found by the kernel test robot and
> recompiled the code

Expected to be in reverse order, with latest changes first.
ie. Here you would start with v8

> v3: tends out the error persisted in the second version. this version is
> a correction of that

?

> 
> v4: did some corrections as recommended by Greg KH

State what changed. Do not expect your review to go back
hunting for past review comments.

Same for all below.

> 
> v5: shortend changelog comments as recommended by Greg KH
> 
> v6: did some corrections as recommended by Greg KH
> 
> v7: fixed some errors on my changelog comments
> 
> v8: fixed some errors pointed out by Philipp Hortmann
> 
> Tanjuate Brunostar (6):
>   staging: vt6655: fix lines ending in a '('

Move ( to end of line

>   staging: vt6655: fix long lines of code in s_uGetRTSCTSDuration

This doesn't match what was actually done

>   staging: vt6655: fix long lines of code in s_uFillDataHead
>   staging: vt6655: fix long lines of code in s_vGenerateTxParamete
>   staging: vt6655: fix long lines of code in the rest of the file

Each commit msg needs to stand alone. The one above only makes
sense (still it's a poor message) when viewed in this patchset.
Once the patches are applied, it must stand alone.

The commit msg should read like a directive. Fix is too general.
Commit msg states what you did, commit log states why you did it.

I see you took 'refactoring' out of the commit messages, but left it
in the commit logs. It doesn't belong in the logs. As another
reviewer pointed out, this is not a refactor.

Alison
> 
>  drivers/staging/vt6655/rxtx.c | 542 ++++++++++++++++++++--------------
>  1 file changed, 324 insertions(+), 218 deletions(-)
> 
> -- 
> 2.34.1
> 
>
  

On Fri, Oct 28, 2022 at 7:11 PM Alison Schofield
<alison.schofield@intel.com> wrote:
>
> We know it's a patchset or series, saying so in subject line is
> redundant. Perhaps - 'Checkpatch cleanup in rxtx.c'
>
> On Fri, Oct 28, 2022 at 05:40:52PM +0000, Tanjuate Brunostar wrote:
> > The fixes are similar, mostly long lines splitting. I had to make
> > serveral patches to ease the work of inspectors
>   ^
> Please use a spell checker.
>
> >
> > v2: fixed a compilation error found by the kernel test robot and
> > recompiled the code
>
> Expected to be in reverse order, with latest changes first.
> ie. Here you would start with v8
>
> > v3: tends out the error persisted in the second version. this version is
> > a correction of that
>
> ?
>
> >
> > v4: did some corrections as recommended by Greg KH
>
> State what changed. Do not expect your review to go back
> hunting for past review comments.
>
> Same for all below.
>
> >
> > v5: shortend changelog comments as recommended by Greg KH
> >
> > v6: did some corrections as recommended by Greg KH
> >
> > v7: fixed some errors on my changelog comments
> >
> > v8: fixed some errors pointed out by Philipp Hortmann
> >
> > Tanjuate Brunostar (6):
> >   staging: vt6655: fix lines ending in a '('
>
> Move ( to end of line
>
> >   staging: vt6655: fix long lines of code in s_uGetRTSCTSDuration
>
> This doesn't match what was actually done
>
> >   staging: vt6655: fix long lines of code in s_uFillDataHead
> >   staging: vt6655: fix long lines of code in s_vGenerateTxParamete
> >   staging: vt6655: fix long lines of code in the rest of the file
>
> Each commit msg needs to stand alone. The one above only makes
> sense (still it's a poor message) when viewed in this patchset.
> Once the patches are applied, it must stand alone.
>
> The commit msg should read like a directive. Fix is too general.
> Commit msg states what you did, commit log states why you did it.
>
> I see you took 'refactoring' out of the commit messages, but left it
> in the commit logs. It doesn't belong in the logs. As another
> reviewer pointed out, this is not a refactor.
>
> Alison
The commit logs I changed  too but they did not show in the
patches when I sent them. I don't get how that happened.
Thanks for the corrections. like i said, I will be sticking with single patches
for now.
Thanks,
Tanju
> >
> >  drivers/staging/vt6655/rxtx.c | 542 ++++++++++++++++++++--------------
> >  1 file changed, 324 insertions(+), 218 deletions(-)
> >
> > --
> > 2.34.1
> >
> >
>
  

On Fri, Oct 28, 2022 at 05:40:52PM +0000, Tanjuate Brunostar wrote:
> The fixes are similar, mostly long lines splitting. I had to make
> serveral patches to ease the work of inspectors
> 
> v2: fixed a compilation error found by the kernel test robot and
> recompiled the code
> 
> v3: tends out the error persisted in the second version. this version is
> a correction of that
> 
> v4: did some corrections as recommended by Greg KH
> 
> v5: shortend changelog comments as recommended by Greg KH
> 
> v6: did some corrections as recommended by Greg KH
> 
> v7: fixed some errors on my changelog comments
> 
> v8: fixed some errors pointed out by Philipp Hortmann
> 
> Tanjuate Brunostar (6):
>   staging: vt6655: fix lines ending in a '('
>   staging: vt6655: fix long lines of code in s_uGetRTSCTSDuration
>   staging: vt6655: fix long lines of code in s_uFillDataHead
>   staging: vt6655: fix long lines of code in s_vGenerateTxParamete
>   staging: vt6655: fix long lines of code in the rest of the file
>   staging: vt6655: fix lines of code ending in a '('
> 

You messed up your patchset...; the shortlog above says "fix longlines"
but individual patch subject says "refactor". Well, refactoring
means resturcturing and improving it without introducing new
functionality or changing behavior, which is really changing existing
code. However, your patchset only do visual formatting, right?

Also, have you ever at least compile-test your patchset (make W=1 and
enabling CONFIG_WERROR)?

Thanks.