Re: [PATCH v2 06/12] scripts: lib: netlink_yml_parser.py: use classes

[Donald Hunter]

On Fri, 13 Jun 2025 at 13:40, Mauro Carvalho Chehab
<mchehab+huawei@xxxxxxxxxx> wrote:
>
> Em Fri, 13 Jun 2025 12:20:33 +0100
> Donald Hunter <donald.hunter@xxxxxxxxx> escreveu:
>
> > Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> writes:
> >
> > > As we'll be importing netlink parser into a Sphinx extension,
> > > move all functions and global variables inside two classes:
> > >
> > > - RstFormatters, containing ReST formatter logic, which are
> > >   YAML independent;
> > > - NetlinkYamlParser: contains the actual parser classes. That's
> > >   the only class that needs to be imported by the script or by
> > >   a Sphinx extension.
> >
> > I suggest a third class for the doc generator that is separate from the
> > yaml parsing.
>
> Do you mean moving those two (or three? [*]) methods to a new class?
>
>     def parse_yaml(self, obj: Dict[str, Any]) -> str:
>     def parse_yaml_file(self, filename: str) -> str:
>     def generate_main_index_rst(self, output: str, index_dir: str) -> None:
>
> Also, how should I name it to avoid confusion with NetlinkYamlParser?
> Maybe YnlParser?

On second thoughts, I see that the rst generation is actually spread
through all the parse_* methods so they are all related to doc generation.

I suggest putting all the parse_* methods into a class called
YnlDocGenerator, so just the 2 classes.

And I'm hoping that generate_main_index_rst can be removed.

> [*] generate_main_index_rst is probably deprecated. eventually
>     we may drop it or keep it just at the command line stript.
>
> > The yaml parsing should really be refactored to reuse
> > tools/net/ynl/pyynl/lib/nlspec.py at some point.
>
> Makes sense, but such change is out of the scope of this series.

Agreed

Thanks,
Donald.