Re: [PATCH v3] Staging: rtl8723bs: Remove spaces before tabs in rtw_set_auth

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 26, 2024 at 02:05:19PM +0200, meir elisha wrote:
> Hi Dan
> 
> Thanks for the response.
> Not sure I got the problem here.
> In V2 I removed spaces and deleted dead code. In V3 I just removed
> spaces after tabs (reverting the dead code changes).
> I'll want to create a seperate patch for the dead code deletion later on.
> What am I missing here?

When I'm reviewing patches, I try to be as machine like as possible.
This makes the review process more predictable.  It helps me avoid
decision fatigue.  https://www.google.com/search?q=decision+fatigue
It sucks to constantly be refusing to apply patches because I know it
makes people feel sad, but when it's part of an automatic process then
it's easier.

This patch here is a very mechanical patch which requires very little
thought, either from the person sending the patch or the person
reviewing it.  You've run into one of the common errors where you
cleaned up dead code instead of deleting it and you've recieved the
automatic response back to just delete the dead code instead of cleaning
it up.

If I hadn't responded, Greg would have said the exact same thing.

Even if you sent the patches in the wrong order:
[patch 1] fix white space
[patch 2] delete dead code
I would respond to patch 1 before reading patch 2 so if you fixed the
white space in the dead code it would still trigger an automatic
response.

I don't know why you wouldn't want to delete the dead code before
fixing the white space issues.  If you had just fixed the white space
issues in the alive code and left the dead code alone then I wouldn't
have noticed or complained.  (Best to avoid this option in case someone
*else* complains that there are still checkpatch warnings remaining).

regards,
dan carpenter





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux