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

John Snow <jsnow@redhat.com> writes:

> 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.

Thanks!

> 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.

Use your judgement.



  reply	other threads:[~2020-09-25  7:50 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
2020-09-25  7:49               ` Markus Armbruster [this message]
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=87eemq1ek5.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jsnow@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.