From: John Snow <email@example.com>
To: Markus Armbruster <firstname.lastname@example.org>
Cc: Michael Roth <email@example.com>,
firstname.lastname@example.org, Eduardo Habkost <email@example.com>,
Cleber Rosa <firstname.lastname@example.org>
Subject: Re: [PATCH v4 02/19] flake8: Enforce shorter line length for comments and docstrings
Date: Tue, 20 Apr 2021 14:06:47 -0400 [thread overview]
Message-ID: <email@example.com> (raw)
On 4/17/21 6:52 AM, Markus Armbruster wrote:
> John Snow <firstname.lastname@example.org> writes:
>> On 4/16/21 8:44 AM, Markus Armbruster wrote:
>>> John Snow <email@example.com> writes:
>> It will be an eventual thing, though: I think we need to agree on a
>> style guide document and in that same series, fix up the instances of
>> defying that guide. I think it's important to pair that work, because
>> the ease of finding and fixing those style deviations will help inform
>> how pragmatic the style guide is.
> Makes sense.
> The introduction of "sphinxy" doc strings (starting with commit
> adcb9b36c) may have been premature.
Somewhat premature, but what other format is there? It would have been
worse to adopt Numpy or google style.
We'll dial it in over time, it will be fine.
>> I feel like it's something I want to do very soon, but not right now.
>> Maybe during the next freeze we can tackle it?
> Whenever you're ready.
> Until then, I feel we should try to minimize doc string churn. Leave
> existing doc strings alone unless they're harmful. Add new ones only
> when we believe they're helpful enough to justify some churn later.
OK. After the expr comments, I actually didn't add very many. I think I
add one or two for the parser because I had trouble understanding at a
glance how it worked, but most of the tiny functions and helpers I left
I barely touched schema.py, because it was complex and I had some
visions of refactoring it a little to make some of the typing better later.
>>> Improvement, but mind PEP 8's "You should use two spaces after a
>>> sentence-ending period in multi-sentence comments".
>> How important is this, and why? My existing prejudice is that it's only
>> a superficial detail of writing with no real impact.
> Holy wars have been fought over less.
>> (Of course, a single space typist WOULD believe that, wouldn't they?
>> Those single-space typists are all the same!)
> I offer three reasons:
> * Local consistency
> * Stick to PEP 8 unless you have good reason not to.
> * It makes Emacs sentence commands work by default.
For me, it's another thing in the category of "I don't actually mind
either way", and can foresee myself accepting a patch using either style
without comment. Inconsistency here doesn't really bother me unless it's
inconsistent within a single docstring.
For QAPI, since you're the maintainer, I can adhere to your style. For
the purposes of all Python code, though, I am not sure I want to bother
enforcing it myself.
You're always welcome to post-edit anything I've written for
typographical consistency as you see fit, I genuinely won't mind. (It
saves me the trouble of having to copy-edit for something I am visually
That said, I'll try to match your preferred style for QAPI at a minimum.
I notice that emacs' reflow command does not always insert two spaces if
a paragraph already sneaks in under the column limit; is there a way to
*force* it to add two spaces?
>> Unfortunately, omitting it from flake8 means I'll probably also miss
>> cases where I or someone else have gone slightly over the limit for
>> docstrings, and doubt it will be enforced consistently.
> I'm happy to correct the occasional minor style issue manually.
If you accept that burden then I have no leg to stand on, I suppose :)
>>> 1.2. you may drop it. I can pick it up and take care of it.
>> This one, please!
> You got it.
Thanks! You can do that whenever, it won't interfere with anything in
next prev parent reply other threads:[~2021-04-20 18:08 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
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 [this message]
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
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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).