All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: Michael Roth <michael.roth@amd.com>,
	Cleber Rosa <crosa@redhat.com>,
	qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [PATCH v4 02/19] flake8: Enforce shorter line length for comments and docstrings
Date: Thu, 08 Apr 2021 10:35:45 +0200	[thread overview]
Message-ID: <87pmz5cgwu.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <33866c5d-1eab-739c-f2e8-a35f4a0b0f55@redhat.com> (John Snow's message of "Fri, 26 Mar 2021 12:30:37 -0400")

John Snow <jsnow@redhat.com> writes:

> On 3/26/21 2:26 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> On 3/25/21 11:21 AM, Markus Armbruster wrote:
>>>> John Snow <jsnow@redhat.com> writes:
>>>>
>>>>> PEP8's BDFL writes: "For flowing long blocks of text with fewer
>>>>> structural restrictions (docstrings or comments), the line length should
>>>>> be limited to 72 characters."
>>>>>
>>>>> I do not like this patch. I have included it explicitly to recommend we
>>>>> do not pay any further heed to the 72 column limit.
>>>>>
>>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>>
>>>> I'd like to get the remainder of this series moving again before digging
>>>> into this patch.
>>>
>>> I am dropping it, then -- I have no interest in bringing a patch I
>>> dislike along for another respin.
>> Despite your dislike, there might be good parts, and if there are,
>> I'd
>> like to mine them.  I don't need you to track the patch for that,
>> though.  Feel free to drop it.
>> Thank you for exploring the max-doc-length option.
>> 
>
> Being less terse about it: Mostly, I don't like how it enforces this
> column width even for indented structures. Generally, we claim that 72 
> columns is "comfortable to read" and I agree.
>
>                                    However, when we start in a margin, I
>                                    am not convinced that this is
>                                    actually more readable than the
>                                    alternative. We aren't using our full
>                                    72 characters here.
>
> For personal projects I tend to relax the column limit to about 100
> chars, which gives nice breathing room and generally reduces the edge 
> cases for error strings and so on. (Not suggesting we do that here so
> long as we remain on a mailing-list based workflow.)
>
> I can't say I am a fan of the limit; I don't think it's something I
> can reasonably enforce for python/* so I have some concerns over 
> consistency, so I think it'd be easier to just not.

I'm with PEP 8 here: go beyond the line length limits juidicously, not
carelessly.

This cannot be enforced automatically with the tools we have.

> I *did* try, though; I just think it brought up too many judgment
> calls for how to make single-line comments not look super awkward. I
> imagine it'll cause similar delays for other authors, and exasperated
> sighs when the CI fails due to a 73-column comment.

Enforcing a hard 72 limit in CI would be precisely what PEP 8 does not
want us to do.



  parent reply	other threads:[~2021-04-08  8:37 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25  6:03 [PATCH v4 00/19] qapi: static typing conversion, pt3 John Snow
2021-03-25  6:03 ` [PATCH v4 01/19] qapi/expr: Comment cleanup John Snow
2021-03-25 15:41   ` Markus Armbruster
2021-03-25 20:06     ` John Snow
2021-03-25  6:03 ` [PATCH v4 02/19] flake8: Enforce shorter line length for comments and docstrings John Snow
2021-03-25 15:21   ` Markus Armbruster
2021-03-25 20:20     ` John Snow
2021-03-26  6:26       ` Markus Armbruster
2021-03-26 16:30         ` John Snow
2021-03-26 16:44           ` Peter Maydell
2021-04-08  8:32             ` Markus Armbruster
2021-04-08  8:58             ` Daniel P. Berrangé
2021-04-09  9:33               ` Markus Armbruster
2021-04-09 17:08                 ` John Snow
2021-04-08  8:35           ` Markus Armbruster [this message]
2021-04-16 12:44   ` Markus Armbruster
2021-04-16 20:25     ` John Snow
2021-04-17 10:52       ` Markus Armbruster
2021-04-20 18:06         ` John Snow
2021-03-25  6:03 ` [PATCH v4 03/19] qapi/expr.py: Remove 'info' argument from nested check_if_str John Snow
2021-03-25  6:03 ` [PATCH v4 04/19] qapi/expr.py: Check for dict instead of OrderedDict John Snow
2021-03-25  6:03 ` [PATCH v4 05/19] qapi/expr.py: constrain incoming expression types John Snow
2021-03-25 14:04   ` Markus Armbruster
2021-03-25 20:48     ` John Snow
2021-03-26  5:40       ` Markus Armbruster
2021-03-26 17:12         ` John Snow
2021-03-25  6:03 ` [PATCH v4 06/19] qapi/expr.py: Add assertion for union type 'check_dict' John Snow
2021-03-25  6:03 ` [PATCH v4 07/19] qapi/expr.py: move string check upwards in check_type John Snow
2021-03-25  6:03 ` [PATCH v4 08/19] qapi: add tests for invalid 'data' field type John Snow
2021-03-25 14:24   ` Markus Armbruster
2021-03-25  6:03 ` [PATCH v4 09/19] qapi/expr.py: Check type of 'data' member John Snow
2021-03-25 14:26   ` Markus Armbruster
2021-03-25 21:04     ` John Snow
2021-03-25  6:03 ` [PATCH v4 10/19] qapi/expr.py: Add casts in a few select cases John Snow
2021-03-25 14:33   ` Markus Armbruster
2021-03-25 23:32     ` John Snow
2021-03-25  6:03 ` [PATCH v4 11/19] qapi/expr.py: Modify check_keys to accept any Collection John Snow
2021-03-25 14:45   ` Markus Armbruster
2021-03-25 23:37     ` John Snow
2021-03-25  6:03 ` [PATCH v4 12/19] qapi/expr.py: add type hint annotations John Snow
2021-03-25  6:03 ` [PATCH v4 13/19] qapi/expr.py: Consolidate check_if_str calls in check_if John Snow
2021-03-25 15:15   ` Markus Armbruster
2021-03-26  0:07     ` John Snow
2021-03-25  6:03 ` [PATCH v4 14/19] qapi/expr.py: Remove single-letter variable John Snow
2021-03-25  6:03 ` [PATCH v4 15/19] qapi/expr.py: enable pylint checks John Snow
2021-03-25  6:03 ` [PATCH v4 16/19] qapi/expr.py: Add docstrings John Snow
2021-04-14 15:04   ` Markus Armbruster
2021-04-17  1:00     ` John Snow
2021-04-17 13:18       ` Markus Armbruster
2021-04-21  1:27         ` John Snow
2021-04-21 13:58           ` Markus Armbruster
2021-04-21 18:20             ` John Snow
2021-03-25  6:03 ` [PATCH v4 17/19] qapi/expr.py: Use tuples instead of lists for static data John Snow
2021-03-25 15:19   ` Markus Armbruster
2021-03-25  6:03 ` [PATCH v4 18/19] qapi/expr.py: move related checks inside check_xxx functions John Snow
2021-03-25  6:03 ` [PATCH v4 19/19] qapi/expr.py: Use an expression checker dispatch table John Snow
2021-03-25 15:46 ` [PATCH v4 00/19] qapi: static typing conversion, pt3 Markus Armbruster
2021-03-26  0:40 ` John Snow
2021-03-26 18:01 ` John Snow

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=87pmz5cgwu.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=michael.roth@amd.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.