Re: [OE-core] [PATCH] Revert "featimage: refactor style"

["Valek, Andrej" <andrej.valek@siemens.com>]

Marek,

Sorry, but these are still not an arguments, why to do that.

On Mon, 2022-01-31 at 10:39 +0100, Marek Vasut wrote:
> On 1/31/22 08:01, Valek, Andrej wrote:
> > Hi,
> 
> Hello Andrej,
> 
> (please avoid top-posting)
> 
> > Sorry, but personally I don't like your idea. What's the benefit of
> > reverting this? I would keep the ${} for bitbake and $ for shell. The
> > {} has to be placed only for variables like $a${b}c.
> 
> That's exactly the benefit of using ${} in shell scripts consistently -
> - 
> you don't have to worry about variable names being accidentally 
> conflated with surrounding strings, either due to your own mistake, or 
> some automated transformation that was applied incorrectly .
> 
> > We should respect the workflow on all recipes otherwise we're braking
> > the "unwritten" rules.
> 
> The workflow on all recipes ? What does this mean ?
> 
> broken by people. Better update the documentation.
> 
> There is one technical counter-argument to this revert from Peter,
> quote:
> "
> There is actually a technical reason to not use ${foo} for shell
> variables unless necessary in bitbake files and it is because
> bitbake will treat them all as potential bitbake variables. This
> means they are unnecessarily included in the taskhashes that
> bitbake calculates.
> "
> 
> But the patch being reverted here addresses the problem only partly, 
> because it still contains remnants like this:
> "
> conf_desc="$conf_desc${sep}setup"
> "
Just for your information, this is not remnants, this is exactly the
right {} usage. If you didn't place the {}, it will be
conf_desc="$conf_desc$sepsetup", which doesn't  make any sense.
> 
> [...]
> 
> > > > > third alternative ? I mean, besides rewriting the fitimage
> > > > > generation into python, which might make it more flexible too.
> > > > 
> > > > Replacing shell code that has grown beyond a couple of hundred
> > > > (tens?) lines with something written in a better language is
> > > > almost always a good idea.
> > > 
> > > It's grown to almost 800 LoC, so maybe it is time to reevaluate the
> > > python conversion then.
> 
> So a rewrite into something more suitable might be a good idea ^ , 
> probably python in this case.

But anyway, if you want to do that, then do not forget for other kernel
classes, where the same commits have been applied by someone else.

So from my point of view, it has to stay like it is.

Cheers,
Andrej