Re: [PATCH maintainer-tools 2/2] dim: Refuse to commit patches that modify files outside of i915

Jani Nikula
2016-04-28T12:48:02Z

On Thu, 28 Apr 2016, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> dim | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/dim b/dim
> index dbcb7d9..1c258be 100755
> --- a/dim
> +++ b/dim
> @@ -691,6 +691,11 @@ function checkpatch_commit
> if test "$bug_lines" -eq 1; then
> warn_or_fail "New BUG macro added"
> fi
> +
> + local non_i915_files=$(git diff-tree --no-commit-id --name-only -r HEAD | grep -v "^\(drivers/gpu/drm/i915/\|include/drm/i915\|include/uapi/drm/i915\)")
> + if [ -n "$non_i915_files" ]; then
> + warn_or_fail "The following files are outside of i915 maintenance scope:\n$non_i915_files\n\nConfirm you have appropriate Acked-by and Reviewed-by for above files"
> + fi

So the original idea beind using warn_or_fail in checkpatch was that it
would get run *before* git am, therefore actually preventing the patch
from being applied. This is no longer the case, because checkpatch fails
to parse base64 encoded emails. Email is hard, so we offload all of that
to git am instead, and check the patch-turned-commit using git show
--pretty=email.

All I'm saying is that the existing use of warn_or_fail in
checkpatch_commit is silly, and adding new ones even more so. I'd just
turn them to direct echos that mimic checkpatch output, with WARNING or
CHECK prefix.

BR,
Jani.

> }
>
> # turn $1 in to a git commit range

--
Jani Nikula, Intel Open Source Technology Center