[oe] [PATCH] Make use of the new bb.utils.filter() function

[Martin Jansa]

On Fri, Mar 03, 2017 at 10:01:50AM +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: openembedded-devel-bounces at lists.openembedded.org
> > [mailto:openembedded-devel-bounces at lists.openembedded.org] On Behalf Of
> > Martin Jansa
> > Sent: den 2 mars 2017 21:49
> > To: Andreas Müller
> > Cc: openembedded-devel at lists.openembedded.org; Peter Kjellerstedt
> > Subject: Re: [oe] [PATCH] Make use of the new bb.utils.filter()
> > function
> > 
> > I agree with Andreas, but I'm not opposed to this so strongly.
> > 
> > Especially as similar patch for oe-core (changing 81 files) was already
> > merged. Similarly the getVar cleanup recently will also cause some
> > headaches when cherry-picking. Unfortunately my "safe" (as it won't
> > break the build) whitespace cleanup won't ever get merged to oe-core, 
> > because it's "too intrusive" :/.
> 
> Being a big fan of whitespaces cleanups I can understand your 
> frustration...

I don't like whitespace cleanups at all, but I hate mixing tabs and
spaces for indentation in the same file even more.

> > Someone else with another opinion?
> > 
> > On Wed, Mar 1, 2017 at 11:02 PM, Andreas Müller <
> > schnitzeltony at googlemail.com> wrote:
> > 
> > > On Wed, Mar 1, 2017 at 6:30 PM, Peter Kjellerstedt
> > > <peter.kjellerstedt at axis.com> wrote:
> > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt at axis.com>
> > >
> > > TBH: I don't like this patch:
> > >
> > > * It combines things which do not share same labels by design. Naming
> > >   package-configs with same label as distro-feature is a nice to have
> > >   but not a design rule.
> > > * To me old style explains better what's going on
> 
> I guess that is a matter of not being used to the new function. I like 
> the fact that in many cases the PACKAGECONFIG definition reduces to as 
> single call to bb.utils.filter().

I agree with Andreas that it was easier to see (for me) what
bb.utils.contains does and it was applicable nicely also for cases where e.g.
x11 in DISTRO_FEATURES enables multiple PACKAGECONFIGs like "x11 themes gtk" while
bb.utils.filter works nicely only for one-to-one cases and for other
cases we still need to use bb.utils.contains, so it's not so consistent.

But if I need to get used to new function and mixing these 2 because of
oe-core, I don't object strongly to use the same everywhere else.

> > > * Heavy-gozilla-patches tend to cause headaches when cherry-picking
> > >   to older release branches. And yes I know we had these patches in 
> > >   the past and nobody complained.
> 
> Well, I can split in in 91 individual patches, if that is preferred...

I don't think this is what Andreas wanted, individual patches might be
easier for me to apply (as the big one currently fails to apply in
master-next).

But having 91 patches doesn't really help when someone
cherry-picks bugfix to older release branches and the bug fix will use
bb.utils.filter or be close to bb.utils.filter to cause conflicts in
cherry-pick.

After current builds are finished and pending master-next changes merged
I plan to put it for test in master-next (will resolve conflict
in big patch manually).

> > > Andreas
> 
> For the record, here is the sed expression I used to switch to the 
> filter function:
> 
> sed -ri -e 's/bb.utils.contains\(\s*(.)([^"'\'']*)\1,\s*(.)([^"'\'']*)\3,\s*(.)\4\5,\s*(.)\6/bb.utils.filter(\1\2\1, \3\4\3/g' **/*(.)
> 
> (It uses ZSH syntax to find all files, which means bash users would 
> have to change it to use find instead.)
> 
> //Peter
> 

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: Digital signature
URL: <http://lists.openembedded.org/pipermail/openembedded-devel/attachments/20170303/576528ad/attachment-0002.sig>