[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 04/21] qapi/parser: factor parsing routine into method
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 04/21] qapi/parser: factor parsing routine into method |
Date: |
Tue, 18 May 2021 11:57:21 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> For the sake of keeping __init__ smaller (and treating it more like a
> gallery of what state variables we can expect to see), put the actual
> parsing action into a parse method. It remains invoked from the init
> method to reduce churn.
I'm kind of *shrug* about this. Well short of "no".
>
> To accomplish this, 'previously_included' becomes the private data
> member '_included', and the filename is stashed as _fname.
Nitpick: you enclose most identifiers in quotes, but not all.
Comments and commit messages often profit from "marking up" identifiers.
Especially when the identifiers are also common words. I like to use
function(), @variable, .attribute, and .method().
>
> Add any missing declarations to the init method, and group them by
> function so they can be understood quickly at a glance.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
- Re: [PATCH v2 01/21] qapi/parser: Don't try to handle file errors, (continued)
- [PATCH v2 06/21] qapi/parser: enforce all top-level expressions must be dict in _parse(), John Snow, 2021/05/11
- [PATCH v2 02/21] qapi: Add test for nonexistent schema file, John Snow, 2021/05/11
- [PATCH v2 03/21] qapi/source: Remove line number from QAPISourceInfo initializer, John Snow, 2021/05/11
- [PATCH v2 07/21] qapi/parser: assert object keys are strings, John Snow, 2021/05/11
- [PATCH v2 05/21] qapi/parser: Assert lexer value is a string, John Snow, 2021/05/11
- [PATCH v2 04/21] qapi/parser: factor parsing routine into method, John Snow, 2021/05/11
- Re: [PATCH v2 04/21] qapi/parser: factor parsing routine into method,
Markus Armbruster <=
- [PATCH v2 09/21] qapi: add must_match helper, John Snow, 2021/05/11
- [PATCH v2 08/21] qapi/parser: Use @staticmethod where appropriate, John Snow, 2021/05/11
- [PATCH v2 10/21] qapi/parser: Fix token membership tests when token can be None, John Snow, 2021/05/11
- [PATCH v2 11/21] qapi/parser: Rework _check_pragma_list_of_str as a TypeGuard, John Snow, 2021/05/11
- [PATCH v2 12/21] qapi/parser: add type hint annotations, John Snow, 2021/05/11
- [PATCH v2 13/21] qapi/parser: Remove superfluous list comprehension, John Snow, 2021/05/11
- [PATCH v2 15/21] qapi/parser: add docstrings, John Snow, 2021/05/11