[PATCH 17/24] binman: fit: Refactor to reduce function size

[Simon Glass]

Hi Alper,

On Tue, 15 Feb 2022 at 04:53, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> On 08/02/2022 21:50, Simon Glass wrote:
> > Split subnode and property processing into separate functions to make
> > the _AddNode() function a little smaller. Tweak a few comments.
> >
> > This does not change any functionality.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
>
> I know this just moves code around a bit, but I think the code here
> could be cleaned up much further with a bit of redesign. I'm not sure of
> the details, but was thinking of at least:
>
> - self._add_fit_image() to handle image/* subnodes
> - self._add_fit_config() to handle configuration/* subnodes
> - self._gen_fdt_nodes() to handle template nodes by calling the above
> - Switching away from recursion to iterating subnodes of fixed nodes

This code is significantly different now, so see what you think.

The last point is interesting...at present you can use property
substitution in any node. I tend to agree the recursion doesn't really
help, but let me know what you think of the latest version.

>
> >
> >  tools/binman/etype/fit.py | 116 ++++++++++++++++++++++++--------------
> >  1 file changed, 73 insertions(+), 43 deletions(-)
> >
> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > index 6ad4a686df..b159844960 100644
> > --- a/tools/binman/etype/fit.py
> > +++ b/tools/binman/etype/fit.py
> > @@ -141,12 +141,82 @@ class Entry_fit(Entry):
> >          super().ReadNode()
> >
> >      def ReadEntries(self):
>
> I think the following functions could be moved out of this one into the
> class scope, even including _AddNode.

OK, will hold off on this one until after the series.

[..]

Regards,
Simon