On Fri, Mar 03, 2017 at 10:01:50AM +0000, Peter Kjellerstedt wrote: > > -----Original Message----- > > From: [email protected] > > [mailto:[email protected]] On Behalf Of > > Martin Jansa > > Sent: den 2 mars 2017 21:49 > > To: Andreas Müller > > Cc: [email protected]; 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 < > > [email protected]> wrote: > > > > > On Wed, Mar 1, 2017 at 6:30 PM, Peter Kjellerstedt > > > <[email protected]> wrote: > > > > Signed-off-by: Peter Kjellerstedt <[email protected]> > > > > > > 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: [email protected]
signature.asc
Description: Digital signature
-- _______________________________________________ Openembedded-devel mailing list [email protected] http://lists.openembedded.org/mailman/listinfo/openembedded-devel
