All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Cleber Rosa" <crosa@redhat.com>
Subject: Re: [PATCH 14/37] qapi/common.py: Move comments into docstrings
Date: Thu, 24 Sep 2020 12:31:56 -0400	[thread overview]
Message-ID: <7a58ccb0-8b54-38e9-316d-c8b7690b6ea1@redhat.com> (raw)
In-Reply-To: <87a6xf8b9a.fsf@dusky.pond.sub.org>

On 9/24/20 11:06 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 9/17/20 3:14 PM, Eduardo Habkost wrote:
>>> On Thu, Sep 17, 2020 at 02:44:53PM -0400, John Snow wrote:
>>> [...]
>>>> Having said this, I have not found any tool to date that actually *checks*
>>>> these comments for consistency. The pycharm IDE interactively highlights
>>>> them when it senses that you have made a mistake, but that cannot be worked
>>>> into our CI process that I know of - it's a proprietary checker.
>>>>
>>>> So right now, they're just plaintext that I am writing to approximate the
>>>> Sphinx style until such time as I enable autodoc for the module and
>>>> fine-tune the actual formatting and so on.
> 
> You are deliberately trying to "approximate" Sphinx style, and ...
> 
>>> After applying this series, I only had to make two small tweaks
>>> to make Sphinx + autodoc happy with the docstrings you wrote.
>>> With the following patch, "make sphinxdocs" will generate the
>>> QAPI Python module documentation at docs/devel/qapi.html.
>>> I had to explicitly skip qapi/doc.py because autodoc thinks the
>>> string constants are documentation strings.
>>>
>>
>> Awesome!
> 
> ... actually almost nail it.
> 
> Please mention your choice of style in the commit message.
> 

OK, I'll try to summarize it. I guess I'll do it in this commit message, 
but it's not likely to be too visible in the future that way, depending 
on how you run git blame and what you're looking at.

I want to resurface my other python style patches soon; perhaps a python 
coding style document should go in alongside those patches.

>> I think I am going to delay explicitly pursuing writing a manual for
>> the QAPI parser for now, but it's good to know I am not too far
>> off. I'm going to target the mypy conversions first, because they can
>> be invasive and full of churn.
>>
>> When I get there, though ... I am thinking I should add this as
>> Devel/QAPI Parser.
> 
> Doing "actually Sphinx style" instead of "an approximation of Sphinx
> style" would reduce churn later on.  So, if it's not too distracting...
> 

Yes, I just mean in terms of rebasing, docstrings and signatures fight 
with each other for context and make re-spinning 125 patches a major 
chore. I wasn't prepared to have the debate on if we wanted to add 
Python code into the Sphinx generator and have that entire discussion yet.

So, I figured it would be a separate series afterwards. I mentioned 
somewhere else that I anticipated doing it when I removed the 
"missing-docstring" warning.

I will investigate doing it (using Eduardo's patches) as a starting 
point while reviews continue to filter in. If I figure it out in time, I 
might squash the formatting changes in, but leave the sphinx enablement 
patches themselves out.

--js



  reply	other threads:[~2020-09-24 16:53 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15 22:39 [PATCH 00/37] qapi: static typing conversion, pt1 John Snow
2020-09-15 22:39 ` [PATCH 01/37] python: Require 3.6+ John Snow
2020-09-16  8:42   ` Markus Armbruster
2020-09-16  8:51   ` Daniel P. Berrangé
2020-09-15 22:39 ` [PATCH 02/37] [DO-NOT-MERGE] qapi: add debugging tools John Snow
2020-09-15 22:39 ` [PATCH 03/37] qapi-gen: Separate arg-parsing from generation John Snow
2020-09-15 22:39 ` [PATCH 04/37] qapi: move generator entrypoint into module John Snow
2020-09-16 11:54   ` Markus Armbruster
2020-09-16 14:24     ` John Snow
2020-09-17  7:46       ` Markus Armbruster
2020-09-15 22:39 ` [PATCH 05/37] qapi: Remove wildcard includes John Snow
2020-09-15 22:39 ` [PATCH 06/37] qapi: delint using flake8 John Snow
2020-09-16 12:12   ` Markus Armbruster
2020-09-16 14:29     ` John Snow
2020-09-17  7:54       ` Markus Armbruster
2020-09-17 16:57         ` John Snow
2020-09-18 10:33           ` Markus Armbruster
2020-09-18 18:13             ` John Snow
2020-09-21  7:31               ` Markus Armbruster
2020-09-21 14:50                 ` John Snow
2020-09-15 22:39 ` [PATCH 07/37] qapi: add pylintrc John Snow
2020-09-16 12:30   ` Markus Armbruster
2020-09-16 14:37     ` John Snow
2020-09-17  7:58       ` Markus Armbruster
2020-09-17 17:06         ` John Snow
2020-09-15 22:39 ` [PATCH 08/37] qapi/common.py: Remove python compatibility workaround John Snow
2020-09-16 12:34   ` Markus Armbruster
2020-09-16 14:38     ` John Snow
2020-09-15 22:39 ` [PATCH 09/37] qapi/common.py: Add indent manager John Snow
2020-09-16 15:13   ` Markus Armbruster
2020-09-16 22:25     ` John Snow
2020-09-17  8:07       ` Markus Armbruster
2020-09-17 17:18         ` John Snow
2020-09-18 10:55           ` Markus Armbruster
2020-09-18 16:08             ` John Snow
2020-09-21  7:43               ` Markus Armbruster
2020-09-15 22:40 ` [PATCH 10/37] qapi/common.py: delint with pylint John Snow
2020-09-17 14:15   ` Markus Armbruster
2020-09-17 17:48     ` John Snow
2020-09-18 11:03       ` Markus Armbruster
2020-09-15 22:40 ` [PATCH 11/37] qapi/common.py: Replace one-letter 'c' variable John Snow
2020-09-17 14:17   ` Markus Armbruster
2020-09-17 17:51     ` John Snow
2020-09-15 22:40 ` [PATCH 12/37] qapi/common.py: check with pylint John Snow
2020-09-15 22:40 ` [PATCH 13/37] qapi/common.py: add notational type hints John Snow
2020-09-17 14:32   ` Markus Armbruster
2020-09-17 18:18     ` John Snow
2020-09-17 20:06       ` John Snow
2020-09-18 11:14       ` Markus Armbruster
2020-09-18 15:24         ` John Snow
2020-09-15 22:40 ` [PATCH 14/37] qapi/common.py: Move comments into docstrings John Snow
2020-09-17 14:37   ` Markus Armbruster
2020-09-17 18:44     ` John Snow
2020-09-17 19:14       ` Eduardo Habkost
2020-09-17 19:31         ` John Snow
2020-09-24 15:06           ` Markus Armbruster
2020-09-24 16:31             ` John Snow [this message]
2020-09-25  7:49               ` Markus Armbruster
2020-09-25 14:07                 ` John Snow
2020-09-15 22:40 ` [PATCH 15/37] qapi/common.py: split build_params into new file John Snow
2020-09-17 14:42   ` Markus Armbruster
2020-09-17 18:53     ` John Snow
2020-09-17 19:40     ` John Snow
2020-09-15 22:40 ` [PATCH 16/37] qapi: establish mypy type-checking baseline John Snow
2020-09-18 11:55   ` Markus Armbruster
2020-09-18 14:27     ` John Snow
2020-09-21  8:05       ` Markus Armbruster
2020-09-21 14:41         ` John Snow
2020-09-25  1:18         ` Eduardo Habkost
2020-09-18 19:03     ` John Snow
2020-09-21  8:05       ` Markus Armbruster
2020-09-21 14:46         ` John Snow
2020-09-15 22:40 ` [PATCH 17/37] qapi/events.py: add notational type hints John Snow
2020-09-15 22:40 ` [PATCH 18/37] qapi/events.py: Move comments into docstrings John Snow
2020-09-15 22:40 ` [PATCH 19/37] qapi/commands.py: Don't re-bind to variable of different type John Snow
2020-09-15 22:40 ` [PATCH 20/37] qapi/commands.py: add notational type hints John Snow
2020-09-15 22:40 ` [PATCH 21/37] qapi/commands.py: enable checking with mypy John Snow
2020-09-15 22:40 ` [PATCH 22/37] qapi/source.py: add notational type hints John Snow
2020-09-15 22:40 ` [PATCH 23/37] qapi/source.py: delint with pylint John Snow
2020-09-15 22:40 ` [PATCH 24/37] qapi/gen.py: Fix edge-case of _is_user_module John Snow
2020-09-15 22:40 ` [PATCH 25/37] qapi/gen.py: add notational type hints John Snow
2020-09-15 22:40 ` [PATCH 26/37] qapi/gen.py: Enable checking with mypy John Snow
2020-09-15 22:40 ` [PATCH 27/37] qapi/gen.py: Remove unused parameter John Snow
2020-09-15 22:40 ` [PATCH 28/37] qapi/gen.py: update write() to be more idiomatic John Snow
2020-09-15 22:40 ` [PATCH 29/37] qapi/gen.py: delint with pylint John Snow
2020-09-15 22:40 ` [PATCH 30/37] qapi/introspect.py: Add a typed 'extra' structure John Snow
2020-09-15 22:40 ` [PATCH 31/37] qapi/introspect.py: add _gen_features helper John Snow
2020-09-15 22:40 ` [PATCH 32/37] qapi/introspect.py: create a typed 'Node' data structure John Snow
2020-09-15 22:40 ` [PATCH 33/37] qapi/introspect.py: add notational type hints John Snow
2020-09-15 22:40 ` [PATCH 34/37] qapi/types.py: " John Snow
2020-09-15 22:40 ` [PATCH 35/37] qapi/types.py: remove one-letter variables John Snow
2020-09-15 22:40 ` [PATCH 36/37] qapi/visit.py: remove unused parameters from gen_visit_object John Snow
2020-09-15 22:40 ` [PATCH 37/37] qapi/visit.py: add notational type hints John Snow
2020-09-16 22:33 ` [PATCH 00/37] qapi: static typing conversion, pt1 John Snow
2020-09-17 20:22 ` John Snow
2020-09-18  7:50   ` Markus Armbruster
2020-09-18 13:07 ` Philippe Mathieu-Daudé
2020-09-18 14:30   ` 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=7a58ccb0-8b54-38e9-316d-c8b7690b6ea1@redhat.com \
    --to=jsnow@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=peter.maydell@linaro.org \
    --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.