qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PULL for-2.7 0/4] Error reporting patches for 2016-08-08
Date: Tue, 9 Aug 2016 15:21:48 +0800	[thread overview]
Message-ID: <20160809072148.GF16125@al.usersys.redhat.com> (raw)
In-Reply-To: <87eg5z5muk.fsf@dusky.pond.sub.org>

On Mon, 08/08 16:58, Markus Armbruster wrote:
> Automated trivial patch checking at work, woot!
> 
> I dropped no-reply@ec2-52-6-146-230.compute-1.amazonaws.com manually.
> Fam, you might want to add a Reply-to: header.
> 
> no-reply@ec2-52-6-146-230.compute-1.amazonaws.com writes:
> 
> > Hi,
> >
> > Your series seems to have some coding style problems. See output below for
> > more information:
> >
> > Message-id: 1470640761-17579-1-git-send-email-armbru@redhat.com
> > Type: series
> > Subject: [Qemu-devel] [PULL for-2.7 0/4] Error reporting patches for 2016-08-08
> >
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> >
> > BASE=base
> > n=1
> > total=$(git log --oneline $BASE.. | wc -l)
> > failed=0
> >
> > commits="$(git log --format=%H --reverse $BASE..)"
> > for c in $commits; do
> >     echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
> >     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> >         failed=1
> >         echo
> >     fi
> >     n=$((n+1))
> > done
> >
> > exit $failed
> > === TEST SCRIPT END ===
> >
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > From https://github.com/patchew-project/qemu
> >  * [new tag]         patchew/1470640761-17579-1-git-send-email-armbru@redhat.com -> patchew/1470640761-17579-1-git-send-email-armbru@redhat.com
> > Switched to a new branch 'test'
> > ab5847c error: Fix error_printf() calls lacking newlines
> > 51b0c39 vfio: Use error_report() instead of error_printf() for errors
> > 5c88cff checkpatch: Fix newline detection in error_setg() & friends
> > b194612 error: Strip trailing '\n' from error string arguments (again)
> >
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/4: error: Strip trailing '\n' from error string arguments (again)...
> > Checking PATCH 2/4: checkpatch: Fix newline detection in error_setg() & friends...
> > ERROR: code indent should never use tabs
> > #27: FILE: scripts/checkpatch.pl:2517:
> > +^I^I^I^Ierror_setg_file_open|$
> >
> > ERROR: code indent should never use tabs
> > #29: FILE: scripts/checkpatch.pl:2519:
> > +^I^I^I^Ierror_prepend|$
> >
> > ERROR: code indent should never use tabs
> > #30: FILE: scripts/checkpatch.pl:2520:
> > +^I^I^I^Ierror_reportf_err|$
> >
> > ERROR: code indent should never use tabs
> > #35: FILE: scripts/checkpatch.pl:2524:
> > +^Iif ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {$
> >
> > total: 4 errors, 0 warnings, 15 lines checked
> >
> > Your patch has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.
> >
> > Checking PATCH 3/4: vfio: Use error_report() instead of error_printf() for errors...
> > Checking PATCH 4/4: error: Fix error_printf() calls lacking newlines...
> > === OUTPUT END ===
> >
> > Test command exited with code: 1
> 
> I refrained from expanding tabs in the lines I touched.  26 out of 35
> patches touching checkpatch.pl add tabs.  1911 out of 2615
> checkpatch.pl's lines contain tabs.
> 
> Perhaps we should suppress this error for checkpatch.pl itself.  Or we
> should make up our minds once and for all to expand tabs there.

Not worth enough to pollute the git blame output IMO.

Perhaps tabs can be ignored by checkpatch.pl when the surrounding untouched
code uses tabs already? That's not an ultimate solution to the tab vs space
dilemma when writing patches to such files, but this seems a relatively
common pattern of ignored warning.

Fam

  reply	other threads:[~2016-08-09  7:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-08  7:19 [Qemu-devel] [PULL for-2.7 0/4] Error reporting patches for 2016-08-08 Markus Armbruster
2016-08-08  7:19 ` [Qemu-devel] [PULL for-2.7 1/4] error: Strip trailing '\n' from error string arguments (again) Markus Armbruster
2016-08-08  7:19 ` [Qemu-devel] [PULL for-2.7 2/4] checkpatch: Fix newline detection in error_setg() & friends Markus Armbruster
2016-08-08  7:19 ` [Qemu-devel] [PULL for-2.7 3/4] vfio: Use error_report() instead of error_printf() for errors Markus Armbruster
2016-08-08  7:19 ` [Qemu-devel] [PULL for-2.7 4/4] error: Fix error_printf() calls lacking newlines Markus Armbruster
2016-08-08  7:29 ` [Qemu-devel] [PULL for-2.7 0/4] Error reporting patches for 2016-08-08 no-reply
2016-08-08 14:58   ` Markus Armbruster
2016-08-09  7:21     ` Fam Zheng [this message]
2016-08-08 12:56 ` Peter Maydell

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=20160809072148.GF16125@al.usersys.redhat.com \
    --to=famz@redhat.com \
    --cc=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).