Re: [PATCH] dt-bindings: pwm: drop unneeded quotes

From: Rob Herring <robh@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Anson Huang <anson.huang@nxp.com>,
	linux-arm-kernel@lists.infradead.org, linux-pwm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: pwm: drop unneeded quotes
Date: Wed, 21 Jun 2023 14:53:17 -0600	[thread overview]
Message-ID: <20230621205317.GA3493078-robh@kernel.org> (raw)
In-Reply-To: <20230612093315.gbabepl5qg44xf5d@pengutronix.de>

On Mon, Jun 12, 2023 at 11:33:15AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Jun 09, 2023 at 04:07:09PM +0200, Krzysztof Kozlowski wrote:
> > Cleanup bindings dropping unneeded quotes. Once all these are fixed,
> > checking for this can be enabled in yamllint.
> 
> in my book quoting everything instead of dropping quotes is the better
> option. While that policy adds more quotes, it prevents surprises like:
> 
> 	$ yaml2json << EOF
> 	> countrycodes:
> 	>  - de
> 	>  - fr
> 	>  - no
> 	>  - pl
> 	> EOF
> 	{
> 	  "countrycodes": [
> 	    "de",
> 	    "fr",
> 	    false,
> 	    "pl"
> 	  ]
> 	}
> 
> And if you use the "only-when-needed" rule of yamllint you have to write
> the above list as:
> 
> 	countrycodes:
> 	 - de
> 	 - fr
> 	 - "no"
> 	 - pl
> 
> which is IMHO really ugly.

Agreed, but "no" and "yes" are unlikely values in DT.

> 
> Another culprit is "on" (which is used e.g. in github action workflows),
> so yamllint tells for example for
> https://github.com/pengutronix/microcom/blob/main/.github/workflows/build.yml:
> 
> 	  3:1       warning  truthy value should be one of [false, true]  (truthy)
> 
> and there are still more surprises (e.g. version numbers might be
> subject to conversion to float).

I'll add a meta-schema check for this. 'const' is already limited to 
string or integer. That's missing from 'enum'. I think we can also check 
that all items are the same type as well.

> So at least in my bubble the general
> hint is to *always* quote strings. Note that required: true is also the
> default for yamllint's quoted-strings setting, proably for pitfalls like
> these.

We're so far gone the other direction from quoting everything, that's 
not going to happen. Plus, if I liked everything quoted, I would have 
used JSON.

My preference here is I don't want to care about this in reviews. I want 
yamllint to check it and not have to think about it again.

Rob