[PATCH 2/3] utils: checkstyle.py: Skip title and trailers checkers for pre-commit

[Dan Scally]

Hi Laurent, thanks for the patch

On 10/08/2024 01:58, Laurent Pinchart wrote:
> When running checkstyle.py in a pre-commit hook, there is either no
> commit message at all (when committing staged changes as a new commit),
> or the commit message comes from a previous commit being amended. In
> either case, there's no new commit message yet, and thus nothing to
> validate for the title and trailers checkers. The trailers checker, in
> particular, will always flag an error, making all commits fail.
>
> To fix that, just skip the two checkers during pre-commit.


I like that this is done in a way which facilitates the same thing being done more easily in the 
future if needed.

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>

> ---
>   utils/checkstyle.py | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index aa0731abdb5a..2b1e1f6c1b9e 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -344,11 +344,16 @@ class CommitChecker(metaclass=ClassRegistry):
>       # Class methods
>       #
>       @classmethod
> -    def checkers(cls, names):
> +    def checkers(cls, commit, names):
>           for checker in cls.subclasses:
>               if names and checker.__name__ not in names:
>                   continue
> -            yield checker
> +            if checker.supports(commit):
> +                yield checker
> +
> +    @classmethod
> +    def supports(cls, commit):
> +        return type(commit) in cls.commit_types
>   
>   
>   class CommitIssue(object):
> @@ -357,6 +362,8 @@ class CommitIssue(object):
>   
>   
>   class HeaderAddChecker(CommitChecker):
> +    commit_types = (Commit, StagedChanges, Amendment)
> +
>       @classmethod
>       def check(cls, commit, top_level):
>           issues = []
> @@ -401,6 +408,8 @@ class HeaderAddChecker(CommitChecker):
>   
>   
>   class TitleChecker(CommitChecker):
> +    commit_types = (Commit,)
> +
>       prefix_regex = re.compile(r'^([a-zA-Z0-9_.-]+: )+')
>       release_regex = re.compile(r'libcamera v[0-9]+\.[0-9]+\.[0-9]+')
>   
> @@ -408,11 +417,6 @@ class TitleChecker(CommitChecker):
>       def check(cls, commit, top_level):
>           title = commit.title
>   
> -        # Skip the check when validating staged changes (as done through a
> -        # pre-commit hook) as there is no title to check in that case.
> -        if isinstance(commit, StagedChanges):
> -            return []
> -
>           # Ignore release commits, they don't need a prefix.
>           if TitleChecker.release_regex.fullmatch(title):
>               return []
> @@ -468,6 +472,8 @@ class TitleChecker(CommitChecker):
>   
>   
>   class TrailersChecker(CommitChecker):
> +    commit_types = (Commit,)
> +
>       commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \(".*"\)')
>   
>       coverity_regex = re.compile(r'Coverity CID=.*')
> @@ -1020,7 +1026,7 @@ def check_style(top_level, commit, checkers):
>       issues = 0
>   
>       # Apply the commit checkers first.
> -    for checker in CommitChecker.checkers(checkers):
> +    for checker in CommitChecker.checkers(commit, checkers):
>           for issue in checker.check(commit, top_level):
>               print('%s%s%s' % (Colours.fg(Colours.Yellow), issue.msg, Colours.reset()))
>               issues += 1