[PATCH 1/5] dim: check all commits in dim apply-pull and push-branch

Jani Nikula jani.nikula at linux.intel.com
Fri May 4 14:06:03 UTC 2018


On Fri, 04 May 2018, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> When merging a pull requests there's potentially a long list of
> problematic patches. But currently we only report the first issue and
> then bail out.
>
> The upside here is that without this patch the workflow when
> processing a pull is:
>
> $ dim apply-pull ...
> -> dim refuses, only reports first problem
> $ dim -d apply-pull
> -> you see all the issues, make up your mind to merge or not
> $ dim -f apply-pull
>
> With this patch we have one step less:
>
> $ dim apply-pull
> -> refuses pull, but prints all the issues
> $ dim -f apply-pull
>
> On the implementation:
> - new helper function to check all the patches, that's the only place
>   we now call warn_or_fail
> - rework the check function to check for everything and return the
>   final verdict
>
> v2: Complete rework on Jani's suggestion.
>
> Acked-by: Dave Airlie <airlied at redhat.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Dave Airlie <airlied at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  dim | 46 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/dim b/dim
> index d8288a342352..d2eb4a0cb6e2 100755
> --- a/dim
> +++ b/dim
> @@ -734,10 +734,11 @@ function dim_rebuild_tip
>  # additional patch checks before pushing, e.g. for r-b tags
>  function checkpatch_commit_push
>  {
> -	local sha1 non_dim
> +	local sha1 non_dim rv
>  
>  	sha1=$1
>  	managed_branch=${2:-1}
> +	rv=0
>  
>  	# use real names for people with many different email addresses
>  	author=$(git show -s $sha1 --format="format:%an")
> @@ -747,30 +748,45 @@ function checkpatch_commit_push
>  
>  	# check for author sign-off
>  	if ! git show -s $sha1 | grep -qi "S.*-by:.*\\($author\\|$author_outlook\\)" ; then
> -		warn_or_fail "$sha1 is lacking author of sign-off"
> +		echoerr "$sha1 is lacking author of sign-off"
> +		rv=1
>  	fi
>  
>  	# check for committer sign-off
>  	if ! git show -s $sha1 | grep -qi "S.*-by:.*$committer"  ; then
> -		warn_or_fail "$sha1 is lacking committer of sign-off"
> +		echoerr "$sha1 is lacking committer of sign-off"
> +		rv=1
>  	fi
>  
>  	# check for Link tag
>  	if [[ "$managed_branch" = "1" ]] && ! git show -s $sha1 | grep -qi 'Link:'  ; then
> -		warn_or_fail "$sha1 is lacking of link tag"
> +		echoerr "$sha1 is lacking of link tag"
> +		rv=1
>  	fi
>  
>  	# check for a-b/r-b tag
> -	if git show -s $sha1 | grep -qi '\(reviewed\|acked\)\S*-by:'  ; then
> -		return
> +	if ! git show -s $sha1 | grep -qi '\(reviewed\|acked\)\S*-by:' && \
> +	   ! [[ "$committer" != "$author" ]]; then
> +		echoerr "$sha1 is lacking mandatory review"
> +		rv=1
>  	fi
>  
> -	# check for committer != author
> -	if [[ "$committer" != "$author" ]]; then
> -		return
> -	fi
> +	return $rv
> +}
>  
> -	warn_or_fail "$sha1 is lacking mandatory review"
> +function checkpatch_commit_push_range
> +{
> +	local rv
> +
> +	rv=0
> +
> +	for sha1 in $(git rev-list $@ --no-merges) ; do

I guess I'd pass --no-merges as params too, and maybe that should be
"$@" to be pedantically correct.

But I guess this is good enough for starters,

Reviewed-by: Jani Nikula <jani.nikula at intel.com>


> +		checkpatch_commit_push $sha1 || rv=1
> +	done
> +
> +	if [ $rv == "1" ] ; then
> +		warn_or_fail "issues in commits detected"
> +	fi
>  }
>  
>  # push branch $1, rebuild drm-tip. the rest of the arguments are passed to git
> @@ -788,9 +804,7 @@ function dim_push_branch
>  
>  	committer_email=$(git_committer_email)
>  
> -	for sha1 in $(git rev-list "$branch@{u}..$branch" --committer="$committer_email" --no-merges) ; do
> -		checkpatch_commit_push $sha1
> -	done
> +	checkpatch_commit_push_range "$branch@{u}..$branch" --committer="$committer_email"
>  
>  	git push $DRY_RUN $remote $branch "$@"
>  
> @@ -909,9 +923,7 @@ function dim_apply_pull
>  	echo Pulling $pull_branch ...
>  
>  	git fetch $pull_branch
> -	for sha1 in $(git rev-list "HEAD..FETCH_HEAD" --no-merges) ; do
> -		checkpatch_commit_push $sha1 0
> -	done
> +	checkpatch_commit_push_range "HEAD..FETCH_HEAD"
>  
>  	$DRY git pull $pull_branch

-- 
Jani Nikula, Intel Open Source Technology Center


More information about the dim-tools mailing list