qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Cleber Rosa" <crosa@redhat.com>
Subject: [RFC] Use of the Nacked-by tag by CI scripts
Date: Mon, 9 Dec 2019 09:44:25 +0100	[thread overview]
Message-ID: <4cbaadf8-ae4f-d086-2137-b83d61a5e9a5@redhat.com> (raw)

Hi,

The Nacked-by tag can be used to manually hold a patch for further 
review, or by automatic CI because of failing test.

We often miss travis-ci and shippable failures. These CI provide a easy 
way to send email on failure, we can integrate the Nacked-by use there.

We can easily have patchew script send a Nacked-by tag.

If there is a consensus about using this tag, the following patch can be 
added to Peter's management scripts:
https://git.linaro.org/people/pmaydell/misc-scripts.git/

If we move to another workflow, having this uniform tag can help future 
merging scripts to avoid patch on hold to get automatically merged.

-- >8 --
Subject: make-pullreq: Do not automatically merge NAcked commits

The 'Nacked-by' tag is a polite way of holding a patch for
further review. Reviewers might share their disapproval with
it (see [1]).

CI scripts might NAck a patch if it breaks testing.
QEMU already thought about using this tag for CI by the past
(see [2]).

The patchwork tool already collects this tag (see [3]).

Also, there was a discussion at the last Open Source Summit
about standardizing it ([4]).

Maintainers might miss a such Nacked-by tag. Help them by
providing a last resort check before merging pull requests.

[1] 
https://www.x.org/wiki/Development/Documentation/SubmittingPatches/#index1h1
[2] https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg00196.html
[3] 
http://git.ozlabs.org/?p=patchwork;a=blobdiff;f=apps/patchwork/models.py;h=fa213dc03e;hp=8871df0259e;hb=487b53576f;hpb=a59ebf107d84b
[4] 
https://lore.kernel.org/workflows/CACT4Y+bxPxQ64HEO2uGRkbk9vJSeg64y10Lak4c2K54J7GyFFA@mail.gmail.com/

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
  make-pullreq | 11 +++++++++++
  1 file changed, 11 insertions(+)

diff --git a/make-pullreq b/make-pullreq
index 61c0f1d..fff0b2d 100755
--- a/make-pullreq
+++ b/make-pullreq
@@ -108,6 +108,17 @@ if [ "$bad" = "yes" ]; then
     exit 1
  fi

+# Check no commit contains a nacked-by tag
+for rev in $(git rev-list master..HEAD); do
+    if git log ${rev}^! | grep -iq "Nacked-by:"; then
+        echo "Error: commit ${rev} nacked"
+        bad=yes
+    fi
+done
+if [ "$bad" = "yes" ]; then
+   exit 1
+fi
+
  # Check whether any authors needs to be corrected after SPF rewrites
  if git shortlog --author=qemu-devel@nongnu.org master..HEAD | grep .; then
      echo "ERROR: pull request includes commits attributed to list"
-- 
2.21.0



             reply	other threads:[~2019-12-09  8:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09  8:44 Philippe Mathieu-Daudé [this message]
2019-12-09  9:28 ` [RFC] Use of the Nacked-by tag by CI scripts Aleksandar Markovic
2019-12-09  9:36   ` Paolo Bonzini
2019-12-09 10:12     ` Aleksandar Markovic
2019-12-09 10:37       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4cbaadf8-ae4f-d086-2137-b83d61a5e9a5@redhat.com \
    --to=philmd@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=crosa@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).