check hold label before merging

[Sashan]

No description provided.

[Sashan]

I think the gmerge tool can check a hold flag got cleared before doing a merge.

[beldmit]

It can. But I use a quite old version of this tool, and don't plan an update

[Sashan]

It can. But I use a quite old version of this tool, and don't plan an update

I see. so perhaps we should add a commit/push hook to repository scripts. so it will avoid merging the stuff with unresolved holds (or not enough reviews).

[paulidale]

Better might be preventing merge for any hold label, not just a committer hold.

[mattcaswell]

I see. so perhaps we should add a commit/push hook to repository scripts. so it will avoid merging the stuff with unresolved holds (or not enough reviews).

This would be better. I do not use ghmerge

[t8m]

IMO this is better than nothing.

BTW, I would like to know why people do not use ghmerge. It works very well.

[t8m]

Could you please check all hold: * labels?

[t8m]

Ping @Sashan

[Sashan]

I tested the change using this simple shell snippet:

#! /usr/bin/env bash

WHAT='openssl'
PRNUM=$1

PR_URL_CONTENTS=$(mktemp /tmp/gh.XXXXXX)
PR_API=repos/openssl/$WHAT/pulls/$PRNUM
PR_URL=https://api.github.com/$PR_API
if which gh >/dev/null 2>&1 ; then
    if ! gh api $PR_API >$PR_URL_CONTENTS; then
        echo "Error getting $PR_API"
        exit 1
    fi
else
    if ! wget --quiet $PR_URL -O $PR_URL_CONTENTS; then
        echo "Error getting $PR_URL"
        exit 1
    fi
fi

set -- $(jq -r '[.head.user.login, .head.ref, .head.repo.ssh_url] | join(" ")' $PR_URL_CONTENTS)

TARGET_REPO=$(jq -r '.base.repo.name' $PR_URL_CONTENTS)
RTM_LABEL=$(jq -r '.labels[] | select(.name == "approval: ready to merge") | .name' $PR_URL_CONTENTS)
URGENT_LABEL=$(jq -r '.labels[] | select(.name == "severity: urgent") | .name' $PR_URL_CONTENTS)
HOLD_LABEL=$(jq -r '.labels[] | select(.name|test("hold*")) | .name' $PR_URL_CONTENTS)

echo "Hold Label: $HOLD_LABEL"
if [ "$TARGET_REPO" != "openssl" ]
then
    >&2 echo "Skipping ready to merge check for non-openssl repo"
elif [ -z "$RTM_LABEL" -a -z "$URGENT_LABEL" ]
then
    >&2 echo "This PR has neither the ready to merge or urgent label set, can't merge"
    exit 1
elif [ -n "$HOLD_LABEL" ] 
then
    >&2 echo "This PR has hold label, can't merge"
    exit 1
fi

echo "you are good to go with pr $PRNUM"
exit 0

I took pr 28691 which has a hold label

lifty$ ./test.sh 28691        
Hold Label: hold: committer discussion
This PR has neither the ready to merge or urgent label set, can't merge

and PR 28726 which has no hold label

lifty$ ./test.sh 28726 
Hold Label: 
This PR has neither the ready to merge or urgent label set, can't merge

[t8m]

This was already merged.