On Sun, 26 Jul 2020 22:33:28 +0200 "Michał Mirosław" <mirq-li...@rere.qmqm.pl> wrote:
> On Sun, Jul 26, 2020 at 08:07:48PM +0200, SeongJae Park wrote: > > On Sun, 26 Jul 2020 09:42:06 -0700 Joe Perches <j...@perches.com> wrote: > > > > > On Sun, 2020-07-26 at 17:36 +0200, SeongJae Park wrote: > > > > On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <j...@perches.com> wrote: > > > [] > > > > > I do not want to encourage relatively inexperienced people > > > > > to run checkpatch and submit inappropriate patches. > > > > > > > > Me, neither. But, I think providing more warnings and references is > > > > better for > > > > that. > > > > > > Unfortunately, the inexperienced _do_ in fact run > > > checkpatch on files and submit inappropriate patches. > > > > > > It's generally a time sink for the experienced > > > maintainers to reply. > > > > > > > Simply limiting checks could allow people submitting inappropriate > > > > patches > > > > intorducing new uses of deprecated terms. > > > > > > Tradeoffs... > > > > > > I expect that patches being reviewed by maintainers > > > are preferred over files being inappropriately changed > > > by the inexperienced. > > > > > > Those inappropriate changes should not be encouraged > > > by tools placed in the hands of the inexperienced. > > > > Right, many things are tradeoff. Seems we arrived in the point, though we > > still have different opinions. To summarize the pros and cons of my patch > > from > > my perspective: > > > > Pros 1: Handle future terms deprecated with different reasons and coverages. > > Pros 2: Inappropriate patches are avoided if the submitters carefully read > > the > > warning messages. > > Cons: Careless people could still bother maintainers by not carefully > > reading > > the message and sending inappropriate patches. > > > > To me, the pros still seems larger than the cons. I would like to also > > again > > mention that the maintainer who first reported the problem, Michal, told > > it's > > ok with the explicit messaging. Nonethelss, this is just my opinion. > > > > Attaching the patch addressing your comments for the previous version. The > > changes from the previous version are: > > > > - Make the name of reported terms not too verbose > > - Avoid unnecessary initialization of the reported terms hash > > - Warn multiple deprecated terms in same line > > Hi, > > Maybe you could split the meaning of --subjective and --strict, and > enable those checks only for --subjective? The test is really hard to do > right: you would have to consider the context and not only mere occurrence > of a word (heh, I even wrote 'blacklisted' here, since it really is about > a night/danger analogy and not political/ethical one). I'm concerning if applying the switch and making this patch non-default could reduce the check coverage. Moreover, IMHO, the deprecation rule of the terms that described in the 'coding-style.rst' is not so subjective but clear. Also, the checkpatch's warning/check message for those seems explicit enough to me. And, the deprecated terms feature is not for this specific terms (master/slave/blacklist/whitelist) only but general deprecated terms. Maybe we could add one more rule in the 'deprecated_terms.txt' by adding a comment, say, "Please add only terms that deprecated with clear rules", for avoiding introduce of subjective deprecated terms in future, though. Thanks, SeongJae Park > > Best Regards, > Michał Mirosław
