On Wed, May 24, 2017 at 11:28:12AM +0200, Daniel Vetter wrote:
> We can't check this when applying (since r-b/a-b tags often get added
> afterwards), but we can check this when pushing. This only looks at
> patches authored by the pusher.
> 
> Also update the docs to highlight that review requirements hold
> especially also for bugfixes.
> 
> Cc: Patrik Jakobsson <patrik.r.jakobs...@gmail.com>
> Cc: Lukas Wunner <lu...@wunner.de>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: Christian König <deathsim...@vodafone.de>
> Cc: Sean Paul <seanp...@chromium.org>

I think I acked this on IRC, but just in case, 

Acked-by: Sean Paul <seanp...@chromium.org>

> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  dim          | 42 ++++++++++++++++++++++++++++++++++++++----
>  drm-misc.rst |  4 +++-
>  2 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/dim b/dim
> index baa0b3832314..79738a1b37a0 100755
> --- a/dim
> +++ b/dim
> @@ -340,6 +340,15 @@ function git_branch_exists # branch
>       fi
>  }
>  
> +function git_committer_email
> +{
> +     if ! commiter_email=$(git config --get user.email) ; then
> +             commiter_email=$EMAIL
> +     fi
> +
> +     echo $commiter_email
> +}
> +
>  # get message id from file
>  # $1 = file
>  message_get_id ()
> @@ -632,11 +641,32 @@ function dim_rebuild_tip
>               exit 1
>       fi
>  }
> +
> +# additional patch checks before pushing, e.g. for r-b tags
> +function checkpatch_commit_push
> +{
> +     local sha1
> +
> +     sha1=$1
> +
> +     # check for a-b/r-b tag
> +     if git show -s $sha1 | grep -qi '\(reviewed\|acked\)\S*-by:'  ; then
> +             return
> +     fi
> +
> +     # check for commiter != author
> +     if [[ $(git show -s $sha1 --format="format:%ce") != $(git show -s $sha1 
> --format="format:%ae") ]]; then
> +             return
> +     fi
> +
> +     warn_or_fail "$sha1 is lacking mandatory review"
> +}
> +
>  # push branch $1, rebuild drm-tip. the rest of the arguments are passed to 
> git
>  # push.
>  function dim_push_branch
>  {
> -     local branch remote
> +     local branch remote commiter_email
>  
>       branch=${1:?$usage}
>       shift
> @@ -645,6 +675,12 @@ function dim_push_branch
>  
>       remote=$(branch_to_remote $branch)
>  
> +     commiter_email=$(git_committer_email)
> +
> +     for sha1 in $(git rev-list $branch@{u}..$branch 
> --committer="$commiter_email" --no-merges) ; do
> +             checkpatch_commit_push $sha1
> +     done
> +
>       git push $DRY_RUN $remote $branch "$@"
>  
>       update_linux_next $branch drm-intel-next-queued drm-intel-next-fixes 
> drm-intel-fixes
> @@ -690,9 +726,7 @@ function dim_apply_branch
>  
>       message_id=$(message_get_id $file)
>  
> -     if ! commiter_email=$(git config --get user.email) ; then
> -             commiter_email=$EMAIL
> -     fi
> +     commiter_email=$(git_committer_email)
>  
>       patch_from=$(grep "From:" "$file" | head -1)
>       if [[ "$patch_from" != *"$commiter_email"* ]] ; then
> diff --git a/drm-misc.rst b/drm-misc.rst
> index caba8dc77696..d56c3c7f92a3 100644
> --- a/drm-misc.rst
> +++ b/drm-misc.rst
> @@ -90,7 +90,9 @@ Merge Criteria
>  Right now the only hard merge criteria are:
>  
>  * Patch is properly reviewed or at least Ack, i.e. don't just push your own
> -  stuff directly.
> +  stuff directly. This rule holds even more for bugfix patches - it would be
> +  embarrassing if the bugfix contains a small gotcha that review would have
> +  caught.
>  
>  * drm-misc is for drm core (non-driver) patches, subsystem-wide refactorings,
>    and small trivial patches all over (including drivers). For a detailed 
> list of
> -- 
> 2.11.0

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to