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.jakobsson@xxxxxxxxx>
> Cc: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Christian König <deathsimple@xxxxxxxxxxx>
> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
I think I acked this on IRC, but just in case,
Acked-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
