Re: [PATCH] Staging: rtl8712: rtl871x_ioctl_linux.c Move constant to right side of the comparision

[Sudip Mukherjee]

On Mon, Sep 28, 2015 at 09:56:19AM -0500, Larry Finger wrote:
> On 09/28/2015 06:18 AM, Sudip Mukherjee wrote:
> >On Sat, Sep 26, 2015 at 12:45:46PM -0500, Larry Finger wrote:
> >>On 09/26/2015 11:49 AM, Punit Vara wrote:
> >>>This patch is to the rtl871x_ioctl_linux.c that fixes up following
> >>>warning reported by checkpatch.pl :
> >>>
> >>>- Comparisons should place the constant on the right side of the test
> >>>
> >>>Signed-off-by: Punit Vara <punitvara@xxxxxxxxx>
> >>
> >>This warning is crap. WTF difference does it make???? The compiler
> >>does not care, and any reader with any piece of a brain is not going
> >>to be confused!
> >>
> >>This patch and all others like it are just meaningless source churning!
> >>
> >>This author has made such a royal mess of his patches that I
> >>recommend that ALL of them be dropped. In addition, we should
> >>continue to drop his changes until he learns how to use git to
> >>generate N/M patches, and until he reads the documentation on patch
> >>submission.
> >Excuse me for my ignorance, but I still can not see what was wrong with
> >his patch. checkpatch is giving warning and he has fixed it. As far as
> >sending in series is concerned, he is a newbie and after telling him how
> >to generate patches in series he has learnt that. I have already told
> >him that his patches might be dropped as they are not in series and he
> >is ready to resend in series as soon as Greg confirms that they are
> >dropped. And as long as the driver is in staging there will be source
> >churning, isn't it?
> >If i remember correctly I was told that for a driver to be moved out of
> >staging the primary thing is that all checkpatch warnings needs to fixed.
> >So if this driver has to move out of staging someday then these warnings
> >also has to be fixed by someone.
> 
> The primary requirement for moving a driver out of staging is that
> it use mac80211. No amount of cosmetic fixing will ever make that
> change for rtl8712u!
Yes, ofcourse. But you will also have to admit that since this is in
staging and newbies are encouraged to first work with staging to fix
checkpatch warnings, so you will get these kind of patches. Outreachy is
starting from tomorrow and I am sure Greg will be flooded with these
kind of patches.
> 
> In my opinion, not ALL checkpatch warnings ever need to be
> heeded.Can you tell me why
> 
> /**
>  * This is a comment
>  */
> 
> is superior to
> 
> /*
>   This is a comment also
>  */
> 
> The difference is not significant, yet checkpatch treats it as
> though the source was horribly flawed. Similarly, satisfying the
> 80-character requirement can leave the code horribly unreadable.
Yes, I admit. Just recently I submited some memory leak patches to another
subsystem and there I had more than 80 char, so just kept a comment that
this warning is not taken care of as it improves readability. But like I
just told this is staging and unless all checkpatch warnings are fixed
someone or the other will continue to send these patches.
> 
> My main complaint is that the OP submitted dozens of patches with
> similar subjects, yet gave no indication is any of these were
> resubmissions, and completely failed to utilize the patch series
> mechanism of git. This behavior makes life difficult for both the
> maintainer and the reviewer.
Yes. True. Even some time he has sent exactly same patch more than one
time, sometimes a new version with no version in subject. That was
confusing. But again he is a newbie. By the time he was informed to send
in series he has already sent lots of patches. I understand your
irritation for that but again he is a newbie. So first few mistakes are
excusable. Isn't it?

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/