Re: [PATCH v3 0/8] PMBus fixes and new functions

["Alex Bennée" <alex.bennee@linaro.org>]

Titus Rwantare <titusr@google.com> writes:

> On Mon, 23 Oct 2023 at 12:16, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> You seem to have missed a number of tags from previous postings:
>>
>>   https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
>>
>> (although I notice we only mention Reviewed-by in the docs)
>>
>> You can use a tool like b4 to apply a series and collect the tags. It
>> will also collect the Message-Id's which are also useful.
>>
>> Once you've fixed up your tags I think as the maintainer you can submit
>> a pull request.
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>
> Thanks for the tip about b4, I spent some time getting to grips with
> it and it's a huge timesaver.
> I haven't quite figured out how to get it to produce and send a signed
> pull request, but I don't need that just yet.

A pull request is really just a GPG signed tag that you push to a repo.
You can use the existing git tooling to create the cover letter for it.

I've included my exact steps at the end of the email but really it comes
down to:

  git tag --sign your-pr-tag
  git push your-pr-tag
  git format-patch <series details>
  git request-pull origin/master your_repo_details your-pr-tag

and finally

  git send-email

My personal exact steps are integrated with my editor but are:

1 Create a branch (optional)
════════════════════════════

  ┌────
  │ git fetch origin
  │ branch="pr/$(date +%d%m%y)-${series}-${version}"
  │ if test -z "$prefix" ; then
  │     git checkout -b $branch origin/master
  │ else
  │     git checkout -b $branch HEAD
  │ fi
  └────


2 Apply patches from mailing list
═════════════════════════════════

  ┌────
  │ if test -z $prefix; then
  │     mkdir -p "${series}.patches"
  │     b4 am -S -t -o "${series}.patches" "${id}"
  │ else
  │     echo "Built tree by hand"
  │ fi
  └────

  ┌────
  │ if test -z $prefix; then
  │     git am ${series}.patches/*.mbx
  │     rm -rf ${series}.patches
  │ else
  │     git log --oneline origin/master..
  │ fi
  └────


3 Check if we are missing any review/comments or tags
═════════════════════════════════════════════════════


4 Check for unreviewed patches
══════════════════════════════


5 Check all sign-offs are there
═══════════════════════════════


6 Check we have only 1 Message-Id per commit
════════════════════════════════════════════


7 Check commits are good
════════════════════════

  We need to ensure we have added our signoff and there is no — ephemera
  left from commit history.

  ┌────
  │ errors=0
  │ commits=0
  │ while read rev; do
  │     author=$(git show -s --format='%an <%ae>' $rev)
  │     body=$(git show -s --format='%B' $rev)
  │     title=$(git show -s --format='%s' $rev)
  │ 
  │     # Check for Author signoff
  │     if ! grep -q "^Signed-off-by: $author" <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"missing author signoff - %h - %an: %s" $rev)
  │     fi
  │ 
  │     # Check for my signoff (fix for quotes)
  │     if ! grep -q "^Signed-off-by: $signoff" <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"missing my signoff - %h - %an: %s" $rev)
  │     fi
  │ 
  │     if ! grep -q "^Message-Id: " <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"missing message id - %h - %an: %s" $rev)
  │     fi
  │ 
  │     # check for unreviewed patches for patches I authored
  │     if [ "$author" = "$signoff" ]; then
  │         if ! grep -q "^Reviewed-by:" <<< "$body"; then
  │             echo $(git log -1 --pretty=format:"unreviewed - %h - %an: %s" $rev)
  │         fi
  │     fi
  │ 
  │     # Check for stray Based-on tags
  │     if grep -q "^Based-on: " <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"has based-on tag - %h - %an: %s" $rev)
  │     fi
  │ 
  │     # Check for stray history
  │     if grep -q "^--" <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"has commit history - %h - %an: %s" $rev)
  │     fi
  │ 
  │     commits=$((commits+1))
  │ done < <(git rev-list "origin/master..HEAD")
  │ 
  │ echo "Found $errors errors over $commits commits"
  └────


8 Preparing a QEMU Pull Request
═══════════════════════════════

  We have two properties here, tversion for the tag and pversion for the
  iteration of the PULL. This is to account for re-rolls where we detect
  and issue after tagging but before we send the PR itself.

  ┌────
  │ (let ((tag (format
  │             "pull-%s-%s-%d"
  │             series
  │             (format-time-string "%d%m%y")
  │             tversion)))
  │   (magit-tag-create tag "HEAD" "--sign")
  │   tag)
  └────

  ┌────
  │ set -e
  │ tag=$(git describe)
  │ git push github $tag
  │ if test -z $prefix; then
  │     git push-ci-now gitlab $tag
  │ else
  │     git push-ci gitlab $tag
  │ fi
  │ echo "pushed $tag"
  └────

  ┌────
  │ (if (= 1 pversion)
  │     "PULL"
  │   (format "PULL v%d" pversion))
  └────

  ┌────
  │ if [ -d "${series}.pull" ]; then
  │   rm -rf ${series}.pull
  │ fi
  │ git format-patch --subject-prefix="$subjprefix" --cover-letter origin/master..HEAD -p -o ${series}.pull
  └────

  You can use the $pull macro to fill in the details


9 And send the pull request
═══════════════════════════

  Using the prefix will limit the send to just the cover letter, useful
  for v2+ pull requests

  ┌────
  │ if test -z "$prefix" ; then
  │   git send-email --confirm=never --dry-run --quiet ${mailto} ${series}.pull/*
  │ else
  │   git send-email --confirm=never --dry-run --quiet ${mailto} ${series}.pull/0000*
  │ fi
  └────

  ┌────
  │ if test -z "$prefix" ; then
  │   git send-email --confirm=never --quiet ${mailto} ${series}.pull/*
  │ else
  │   git send-email --confirm=never --quiet ${mailto} ${series}.pull/0000*
  │ fi
  │ rm -rf ${series}.pull
  └────

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro