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

Daniel Vetter daniel.vetter at ffwll.ch
Fri May 4 13:32:34 UTC 2018


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
+		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
 
-- 
2.17.0



More information about the dim-tools mailing list