> -----Original Message-----
> From: Marek Vasut <[email protected]>
> Sent: den 29 januari 2022 02:39
> To: Peter Kjellerstedt <[email protected]>; openembedded-
> [email protected]
> Cc: Andrej Valek <[email protected]>; Richard Purdie
> <[email protected]>
> Subject: Re: [OE-core] [PATCH] Revert "featimage: refactor style"
> 
> On 1/29/22 02:06, Peter Kjellerstedt wrote:
> >> -----Original Message-----
> >> From: [email protected] <openembedded-
> >> [email protected]> On Behalf Of Marek Vasut
> >> Sent: den 29 januari 2022 01:29
> >> To: [email protected]
> >> Cc: Marek Vasut <[email protected]>; Andrej Valek
> <[email protected]>;
> >> Richard Purdie <[email protected]>
> >> Subject: [OE-core] [PATCH] Revert "featimage: refactor style"
> >>
> >> This reverts commit f44bb458884da64356ee188917094b5515d3b159.
> >>
> >> The reverted patch attempted to perform some sort of clean up, however
> >> it only brought in style inconsistencies like this:
> >>
> >> ```
> >> conf_desc="$conf_desc${sep}setup"
> >> ```
> >>
> >> The curly brackets around variables were placed in the kernel-fitimage
> >> bbclass deliberately, since when assembling the fitimage ITS there are
> >> multiple variables where it is difficult to identify where the variable
> >> ends and some sort of follow up string starts.
> >
> > 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.
> 
> Yikes. (it would be good to include this gem in the commit message)

Well there was:

    - use bash variable notation without {} where possible
      - just to make sure it looks like bash variable not bitbake variable one

though I guess my explanation has a bit more technical merit 
than that it improves the looks. ;)

> So are we stuck with this inconsistent coding style change 
> or is there a

Personally I do not see it as inconsistent, it is just the way 
shell handles variables. It is just something to get used to (I 
also had a colleague who would review any shell code changes we 
made and comment on every single unnecessary character so one 
quickly learned to use $foo everywhere possible rather than 
${foo}...) That said, I have never looked at this code so I 
have no real idea of how bad it is or isn't.

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

//Peter

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#161091): 
https://lists.openembedded.org/g/openembedded-core/message/161091
Mute This Topic: https://lists.openembedded.org/mt/88758521/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to