Re: [Buildroot] [PATCH 000/102] Standardize/beautify genimage .cfg files for all boards

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 000/102] Standardize/beautify genimage .cfg files for all boards
Date: Mon, 8 Nov 2021 16:24:04 +0100	[thread overview]
Message-ID: <20211108162404.58ebbd3c@windsurf> (raw)
In-Reply-To: <20211108150731.GD680793@scaer>

Hello,

On Mon, 8 Nov 2021 16:07:31 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> I know this as been discussed on another thread with Peter, about using
> K/M instead of bare numbers, and it was my understanding that if a
> change were to be made, it would be restricted to just that.
> 
> However, I question the relevance of this *huge* patchset just for a
> makeover.
> 
> Yes, usually, I like consistency, but here we have a two-edge issue:
> 
>   - the series is huge, and causes a lot of churn making pending patches
>     more difficult to apply,

I would agree in general, but here I doubt it is really going to cause
much conflict with other pending patches.

>   - the genimage format is not well defined, and there is no linter, so
>     the existing files *will* diverge again, and we won't notice that
>     new files won't be compliant.

Right, but that's not really a reason to not take this patch series.

On my side, I would simply apply the whole thing. It makes things
overall better (perhaps not perfect, but better and more consistent).

> I usually l̶o̶̶a̶t̶h̶ d̶e̶s̶p̶i̶s̶e̶ h̶a̶t̶e̶ do not like TABs... The width of a TAB
> varies depending on one's environment and coding practices; the usual
> 8-space width makes for very long, ugly lines; TAB break carefull
> indentation.

A matter of taste indeed, but in "make" (you know this language we
heavily abuse in Buildroot), TABs are king! So it would be kind of
ironical not to allow TABs in a make-based project :-)

> And then this gave a 107-patch series. And the commits all have the same
> typo: 'beatify' instead of 'beautify'. Although I can forgive a typo
> (who would I be not to?), and this one was funny one, fixing all those
> manually when applying will be a tedious task, though, so you would have
> to do that on your side and respin.

I don't know if that typo is worth a huge respin.

> But before you do so, please wait a bit for others to speak up, if this
> series is really meaningful.

I haven't reviewed it in its entirety, but overall, I think we should
just apply, and move on. I.e, be pragmatic.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot