From: John Snow <firstname.lastname@example.org> To: Markus Armbruster <email@example.com> Cc: Michael Roth <firstname.lastname@example.org>, email@example.com, Eduardo Habkost <firstname.lastname@example.org>, Cleber Rosa <email@example.com> Subject: Re: [PATCH v4 02/19] flake8: Enforce shorter line length for comments and docstrings Date: Fri, 26 Mar 2021 12:30:37 -0400 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> On 3/26/21 2:26 AM, Markus Armbruster wrote: > John Snow <firstname.lastname@example.org> writes: > >> On 3/25/21 11:21 AM, Markus Armbruster wrote: >>> John Snow <email@example.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 <firstname.lastname@example.org> >>> >>> 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 *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.
next prev parent reply index 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 [this message] 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 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.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
QEMU-Devel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \ firstname.lastname@example.org public-inbox-index qemu-devel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git