Re: [PATCH v2] staging: rtl8723bs: fix constant on left side of test checkpatch warnings

[Shuah Khan]

On 3/25/26 15:34, Dan Carpenter wrote:
> On Wed, Mar 25, 2026 at 11:57:38AM -0600, Shuah Khan wrote:
> > My primary concern with this change is that it 10 files changed, with
> > 26 insertions(+), 32 deletions(-)
> >
> > It can be difficult to find any regressions unless the changes are
> > tested. I understand it is staging repo, but if we relax the rules
> > for staging irrespective of the scope of change, it becomes lot
> > harder to find regressions later. We are essentially leaving the
> > job of testing to users. Also if ans when the driver is pulled into
> > mainline, it will inherit the regressions that crept in.
>
> To me this is a mechanical patch (basically automatic, flip the
> if statement) from a newbie.  These kinds of patches are seldom a
> problem.
>
> I took a sample of the patches for five years from 2021 to the end of
> 2025.  We merged 54406 commits in staging.  We had 401 Fixes tags.  But
> then I decided I don't care about Kconfig problems so I cut it down to
> 380 bugs.  Half of those (189 commits) were from when the driver was
> merged.  We tend to not review code very much when it is first merged
> to staging.  The other half (191 commits) were from us missing bugs
> during review.  191 bugs over five years is 40 bugs per year.
>
> I hand reviewed a bunch of the 191 commits.  It's mostly senior
> developers and maintainers who introduce bugs.  They are normally doing
> complicated things like adding features or re-working core parts of the
> driver.  Quite a few of these patches were tested.
>
> The fallout from newbie patches is pretty rare and often really
> minor.  A common thing is "We deleted code but we didn't delete
> everything."  The number of serious mess ups is probably just a
> couple times per year out of the 40 total bugs per year.
>
> I've attached my script so you can check yourself.  NEW means the
> bug was introduced by a new driver.  OLD means, ideally it would
> have been caught in review.

I am not the maintainer for this driver, so it is really up to the
maintainer(s) to accept mechanical patches such as this one.

As for the mentorship, I like to use a consistent rule that testing
for regressions is important and that author needs to disclose how
the patch is tested or not tested in the version 1 of the patch and
add RFT tag. That way maintainers can make an informed decision on
whether to accept the patch or not.

thanks,
-- Shuah