Re: [PATCH v4] checkpatch: add support to check 'Fixes:' tag format

Re: [PATCH v4] checkpatch: add support to check 'Fixes:' tag format

[Markus Elfring]

> The check doesn't support below formats and it will emit diagnostics info for them:
…
> Fixes: ba35f8588f47 (“ipvlan: Defer multicast / broadcast processing to a work-queue”)
…
> Fixes: 03f6fc6de919 ('ASoC: rt5682: Add the soundwire support')

Will the tolerance (and support) grow for such quotation character alternatives?


> Note: this patch also fixes double quotation mark issue for normal git
>       commit description, and now it supports double quotation mark in
>       title line, for example:
>       Commit e33e2241e272 ("Revert "cfg80211: Use 5MHz bandwidth by default
>       when checking usable channels"")

Do you care to achieve a safe data format description also for this use case?


> Note: this patch also adds diagnostics info support for normal git commit
>       description format check.

Does this information indicate a need to split possible changes into
separate update steps?


> +				$diagnostics .= "Missing a pair of parentheses '()' or a pair of double quotation marks (\"\").\n";

Can such a message trigger any more thoughts and development ideas?


> +					$diagnostics .= "The title is too abbreviated, at least half of orignial commit title is necessary.\n";

* Please avoid a typo in this message.

* Which formula do you propose for the length calculation?

Regards,
Markus

Re: [PATCH v4] checkpatch: add support to check 'Fixes:' tag format

[Joe Perches]

On Sat, 2020-05-02 at 21:40 +0200, Markus Elfring wrote:
> > The check doesn't support below formats and it will emit diagnostics info for them:
[]
> Will the tolerance (and support) grow for such quotation character alternatives?

No.

> Does this information indicate a need to split possible changes into
> separate update steps?

No.

> * Which formula do you propose for the length calculation?

None.

Re: [v4] checkpatch: add support to check 'Fixes:' tag format

[Markus Elfring]

>> Will the tolerance (and support) grow for such quotation character alternatives?
>
> No.

Would you prefer to achieve a restrictive data format description?


>> * Which formula do you propose for the length calculation?
>
> None.

I imagine that such a view can increase the probability for confusion.

Regards,
Markus

Re: [PATCH v4] checkpatch: add support to check 'Fixes:' tag format

[Wang YanQing]

On Sat, May 02, 2020 at 09:40:24PM +0200, Markus Elfring wrote:
> 
> 
> > +				$diagnostics .= "Missing a pair of parentheses '()' or a pair of double quotation marks (\"\").\n";
> 
> Can such a message trigger any more thoughts and development ideas?

No, I don't think so. '(" ... ")' is the minimum interface between analyser
(checkpatch) and commit id description (normal commit id and 'Fixes:' tag)
about the title, it is very difficult if not impossible to guess the title
boundary and whether it is the *REAL* title that folllow the SHA1 without
this precondition, and it is more difficult to do it when we need to support
title which across lines in the normal commit id description.

At last I really doubt the benefit it brings deserves the complexity and the
current diagnostics info is enough clear for most situation.

Thanks.

Re: [v4] checkpatch: add support to check 'Fixes:' tag format

[Markus Elfring]

>>> +				$diagnostics .= "Missing a pair of parentheses '()' or a pair of double quotation marks (\"\").\n";
>>
>> Can such a message trigger any more thoughts and development ideas?
>
> No, I don't think so. '(" ... ")' is the minimum interface between analyser
> (checkpatch) and commit id description (normal commit id and 'Fixes:' tag)

* I am proposing to recheck the influence of quotation character alternatives
  also at this place.

* How do you think about to provide an information without the word “or”
  in error messages?


> about the title, it is very difficult if not impossible to guess the title
> boundary and whether it is the *REAL* title that folllow the SHA1 without
> this precondition,

I find this concern reasonable.


> and it is more difficult to do it when we need to support
> title which across lines in the normal commit id description.

I imagine that further software design options can be considered.


> At last I really doubt the benefit it brings deserves the complexity and the
> current diagnostics info is enough clear for most situation.

* Will a safer data format description trigger corresponding efforts?

* Will the structure of the commit title matter?

* Would you like to improve checks (and program organisation) besides
  the application of regular expressions?

Regards,
Markus