On Fri, 25 Sep 2020, Dwaipayan Ray wrote:

> Hi,
> As Joe mentioned earlier, there might be four new
> warnings to handle better:
> 
> 1) Same name, different address
> 2) Same address, different name
> 3) email extensions present in header but
>     not in signed off by
> 4) comment blocks after name
> 
> I am thinking to solve the first three in a single patch.
> 
> As for the email extensions part, should it be handled
> differently? There will be redundant calls to that
> though because it's not an error seen quite often.
>

Dwaipayan, thanks for this initial structuring.

Let us try to put some systematic structure into this handling of the 
NO_AUTHOR_SIGN_OFF.

There are basically two aspects to consider:

- which classes are potential mismatches can we potentially observe?

- which level of severity do we assign to each class of mismatch?

To the first point, you started with some initial classes above.

  0) same name, same address (no mismatch)
  1) same name, different address
     subclasses: (how 'different' address?)
     - email address differ by valid difference, e.g., extensions that 
       will go to the same inbox, e.g.,  the xyz+fds extension in mail 
       addresses. (that is your class 3, right?)
     - two email addresses that are known to belong to the same individual
       - known because of .mailmap
       - known otherwise?
  2) different name, same address
     subclasses: (how 'different' name?)
     - examples:
       - Firstname, Lastname (but middle-name initials differ)
       - special "regional" characters, like ü vs. ue, etc.
       - firstname and lastname are reverted etc.
  3) different name, different address (but still we believe it "matches")
     combinations of subclasses of 1) and 2), which we want to consider.

Then, to the second question:

So, these different classes we can think of and "assign" different 
severities that checkpatch.pl can use.

Checkpatch.pl already has four levels:

three severity reporting levels: ERROR, WARN, CHECK, and
the level _do not report_ (which is implemented by just ignoring some kind 
of difference).

I think a lot of discussion will be around what severity to assign to 
which class above, and in some way, long-term maintainers have a larger 
say than we do here.

So, let us first modify checkpatch.pl to identify the various classes 
above, maybe just starting very basic, with different name, same address
and same name, different address and run it over the commit range and see 
which examples show up and how often.

For now, we first just use checkpatch.pl to identify the different types 
and refine them into subclasses; then, we can discuss with severity should 
be assigned to which type of mismatch.

The second question should not invalidate our data collection and 
identification of subclasses, though.

Does that help? What do you think?


Lukas