[libcamera-devel] [PATCH v2 2/6] checkstyle: Exit with 1 status if issues are found

[Laurent Pinchart]

Hi Nicolas,

Thank you for the patch.

On Fri, Jan 17, 2020 at 02:17:29PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> Makes the tool return 1 if there is any potential issues. This is
> needed when using this tool for pre-commit hook in order to abort
> the commit process.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> ---
>  utils/checkstyle.py | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>  mode change 100755 => 100644 utils/checkstyle.py
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> old mode 100755
> new mode 100644
> index 7edea25..e7375b3
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -541,7 +541,7 @@ def check_style(top_level, commit):
>      files = [f for f in files if len([p for p in patterns if fnmatch.fnmatch(os.path.basename(f), p)])]
>      if len(files) == 0:
>          print("Commit doesn't touch source files, skipping")
> -        return
> +        return 0
>  
>      issues = 0
>      for f in files:
> @@ -554,6 +554,8 @@ def check_style(top_level, commit):
>          print("%u potential style %s detected, please review" % \
>                  (issues, 'issue' if issues == 1 else 'issues'))
>  
> +    return issues
> +
>  
>  def extract_revlist(revs):
>      """Extract a list of commits on which to operate from a revision or revision
> @@ -632,11 +634,12 @@ def main(argv):
>  
>      revlist = extract_revlist(args.revision_range)
>  
> +    issues = 0
>      for commit in revlist:
> -        check_style(top_level, commit)
> +        issues += check_style(top_level, commit)
>          print('')
>  
> -    return 0
> +    return min(issues, 1)

I find this a bit difficult to read, but the python alternative to the C
ternary operator would be

	return issues and 1 or 0

which may not be more reable.

	if issues:
		return 1
	else:
		return 0

may be an option, but not very nice either. I'll leave it up to you,
maybe

	# Return an error if any issues have been detected
	return min(issues, 0)

In any case,

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

>  
>  
>  if __name__ == '__main__':

-- 
Regards,

Laurent Pinchart