Re: [PATCH v2 04/21] qapi/parser: factor parsing routine into method

[John Snow]

On 5/18/21 5:57 AM, Markus Armbruster wrote:
> 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".

Understood. Hopefully it's lateral movement at best to you; I don't want 
to make it distinctly worse in your eyes.

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

Consistency in this regard is ... OK, not my strong suite. It can be 
hard to remember when to use which systems, and I'm caught between md, 
rst, kerneldoc, qapidoc, etc.

More or less, I am always happy if you just edit little things like this 
on pull as you see fit, I won't be mad.

I'll edit this in my local branch for now.

--js

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