QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Connor Kuehl <ckuehl@redhat.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Eric Blake <eblake@redhat.com>,
	qemu-devel@nongnu.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions
Date: Thu, 10 Jun 2021 14:37:02 +0200	[thread overview]
Message-ID: <87o8cd6glt.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <043f0098-d043-b649-9b2d-1ebee6f5f92c@redhat.com> ("Philippe Mathieu-Daudé"'s message of "Thu, 10 Jun 2021 12:15:39 +0200")

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 6/10/21 11:39 AM, Markus Armbruster wrote:
>> Just code motion, so
>> 
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> 
>> Opportunity to wrap the long doc comment lines.  Should be kept under 70
>> or so.
>
> Hmm is that a QAPI specific requirement? It is not enforced by
> checkpatch, and still in the 80-90 grey area:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg763806.html

Consider why we limit line length at all: it's for *legibility*.  Humans
tend to have trouble following long lines with our eyes (I sure do).
Typographic manuals suggest to limit columns to roughly 60 characters
for exactly that reason[1].  Four levels of indentation plus 60
characters of actual text yields 76.

We add a grey area to provide for the occasional case where deeper
indentation pushes code of reasonable width beyond the "white" area, and
breaking these lines would be less legible than making use of the grey
area.

The lines I referred to are long for no good reason.  Wrapping them will
improve legibility.

> (I can do if respin required, but I'd rather have this catch earlier,
> not at code movement).

Before, during, after, or even not at all *clank*[2], your choice.

> Thanks for the A-b!


[1] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style

[2] Sad sound of a can being kicked down the road



  reply	other threads:[~2021-06-10 12:38 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10  6:45 [PATCH 00/11] target/i386/sev: Housekeeping helping using SEV-disabled binaries Philippe Mathieu-Daudé
2021-06-10  6:45 ` [PATCH 01/11] MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV Philippe Mathieu-Daudé
2021-06-15 17:46   ` Philippe Mathieu-Daudé
2021-06-10  6:45 ` [PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions Philippe Mathieu-Daudé
2021-06-10  9:39   ` Markus Armbruster
2021-06-10 10:15     ` Philippe Mathieu-Daudé
2021-06-10 12:37       ` Markus Armbruster [this message]
2021-06-10  6:45 ` [PATCH 03/11] target/i386/monitor: Return QMP error when SEV is disabled in build Philippe Mathieu-Daudé
2021-06-10  8:12   ` Dr. David Alan Gilbert
2021-06-10 14:52   ` Connor Kuehl
2021-06-10  6:45 ` [PATCH 04/11] target/i386/cpu: Add missing 'qapi/error.h' header Philippe Mathieu-Daudé
2021-06-10  8:16   ` Dr. David Alan Gilbert
2021-06-10 14:52   ` Connor Kuehl
2021-06-10  6:45 ` [PATCH 05/11] target/i386/sev_i386.h: Remove unused headers Philippe Mathieu-Daudé
2021-06-10 14:52   ` Connor Kuehl
2021-06-10  6:45 ` [PATCH 06/11] target/i386/sev: Remove sev_get_me_mask() Philippe Mathieu-Daudé
2021-06-10  8:28   ` Dr. David Alan Gilbert
2021-06-10 14:52   ` Connor Kuehl
2021-06-10  6:45 ` [PATCH 07/11] target/i386/sev: Mark unreachable code with g_assert_not_reached() Philippe Mathieu-Daudé
2021-06-10 14:52   ` Connor Kuehl
2021-06-10  6:45 ` [PATCH 08/11] target/i386/sev: sev_get_attestation_report use g_autofree Philippe Mathieu-Daudé
2021-06-10  6:45 ` [PATCH 09/11] target/i386/sev: Restrict SEV to system emulation Philippe Mathieu-Daudé
2021-06-10  6:45 ` [PATCH 10/11] target/i386/monitor: Move SEV specific commands to sev.c Philippe Mathieu-Daudé
2021-06-10 15:19   ` Connor Kuehl
2021-06-16 20:42     ` Philippe Mathieu-Daudé
2021-06-10  6:45 ` [PATCH 11/11] monitor: Restrict 'info sev' to x86 targets Philippe Mathieu-Daudé

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=87o8cd6glt.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=brijesh.singh@amd.com \
    --cc=ckuehl@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=philmd@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 read-only IMAP folder(s) and NNTP newsgroup(s).