Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files
Hi Rob,
On 12/08/20 22:36, Rob Herring wrote:
Clean-up incorrect indentation, extra spaces, long lines, and missing EOF newline in schema files. Most of the clean-ups are for list indentation which should always be 2 spaces more than the preceding keyword.
Found with yamllint (which I plan to integrate into the checks).
[...]
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml index 3d4e1685cc55..28c6461b9a9a 100644 --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml @@ -95,10 +95,10 @@ allOf: # Devices without builtin crystal properties: clock-names:
minItems: 1
maxItems: 2
items:
enum: [ xin, clkin ]
minItems: 1
maxItems: 2
items:
enum: [ xin, clkin ] clocks: minItems: 1 maxItems: 2
Thanks for noticing, LGTM.
[...]
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml index d7dac16a3960..36dc7b56a453 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml @@ -33,8 +33,8 @@ properties: $ref: /schemas/types.yaml#/definitions/uint32
touchscreen-min-pressure:
- description: minimum pressure on the touchscreen to be achieved in order for the
touchscreen driver to report a touch event.
- description: minimum pressure on the touchscreen to be achieved in order
for the touchscreen driver to report a touch event.
Out of personal taste, I find the original layout more pleasant and readable. This third option is also good, especially for long descriptions:
description: minimum pressure on the touchscreen to be achieved in order for the touchscreen driver to report a touch event.
At first glance yamllint seems to support exactly these two by default:
With indentation: {spaces: 4, check-multi-line-strings: true}
the following code snippet would PASS:
Blaise Pascal: Je vous écris une longue lettre parce que je n'ai pas le temps d'en écrire une courte.
the following code snippet would PASS:
Blaise Pascal: Je vous écris une longue lettre parce que je n'ai pas le temps d'en écrire une courte.
the following code snippet would FAIL:
Blaise Pascal: Je vous écris une longue lettre parce que je n'ai pas le temps d'en écrire une courte.
(https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.i...)
On Thu, Aug 13, 2020 at 4:31 AM Luca Ceresoli luca@lucaceresoli.net wrote:
Hi Rob,
On 12/08/20 22:36, Rob Herring wrote:
Clean-up incorrect indentation, extra spaces, long lines, and missing EOF newline in schema files. Most of the clean-ups are for list indentation which should always be 2 spaces more than the preceding keyword.
Found with yamllint (which I plan to integrate into the checks).
[...]
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml index 3d4e1685cc55..28c6461b9a9a 100644 --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml @@ -95,10 +95,10 @@ allOf: # Devices without builtin crystal properties: clock-names:
minItems: 1
maxItems: 2
items:
enum: [ xin, clkin ]
minItems: 1
maxItems: 2
items:
enum: [ xin, clkin ] clocks: minItems: 1 maxItems: 2
Thanks for noticing, LGTM.
[...]
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml index d7dac16a3960..36dc7b56a453 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml @@ -33,8 +33,8 @@ properties: $ref: /schemas/types.yaml#/definitions/uint32
touchscreen-min-pressure:
- description: minimum pressure on the touchscreen to be achieved in order for the
touchscreen driver to report a touch event.
- description: minimum pressure on the touchscreen to be achieved in order
for the touchscreen driver to report a touch event.
Out of personal taste, I find the original layout more pleasant and readable. This third option is also good, especially for long descriptions:
description: minimum pressure on the touchscreen to be achieved in order for the touchscreen driver to report a touch event.
At first glance yamllint seems to support exactly these two by default:
With indentation: {spaces: 4, check-multi-line-strings: true}
Turning on check-multi-line-strings results in 10K+ warnings, so no.
The other issue is the style ruamel.yaml wants to write out is as the patch does above. This matters when doing some scripted transformations where we read in the files and write them back out. I can somewhat work around that by first doing a pass with no changes and then another pass with the actual changes, but that's completely scriptable. Hopefully, ruamel learns to preserve the style better.
Rob
Hi,
On 14/08/20 16:51, Rob Herring wrote:
On Thu, Aug 13, 2020 at 4:31 AM Luca Ceresoli luca@lucaceresoli.net wrote:
Hi Rob,
On 12/08/20 22:36, Rob Herring wrote:
Clean-up incorrect indentation, extra spaces, long lines, and missing EOF newline in schema files. Most of the clean-ups are for list indentation which should always be 2 spaces more than the preceding keyword.
Found with yamllint (which I plan to integrate into the checks).
[...]
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml index 3d4e1685cc55..28c6461b9a9a 100644 --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml @@ -95,10 +95,10 @@ allOf: # Devices without builtin crystal properties: clock-names:
minItems: 1
maxItems: 2
items:
enum: [ xin, clkin ]
minItems: 1
maxItems: 2
items:
enum: [ xin, clkin ] clocks: minItems: 1 maxItems: 2
Thanks for noticing, LGTM.
[...]
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml index d7dac16a3960..36dc7b56a453 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml @@ -33,8 +33,8 @@ properties: $ref: /schemas/types.yaml#/definitions/uint32
touchscreen-min-pressure:
- description: minimum pressure on the touchscreen to be achieved in order for the
touchscreen driver to report a touch event.
- description: minimum pressure on the touchscreen to be achieved in order
for the touchscreen driver to report a touch event.
Out of personal taste, I find the original layout more pleasant and readable. This third option is also good, especially for long descriptions:
description: minimum pressure on the touchscreen to be achieved in order for the touchscreen driver to report a touch event.
At first glance yamllint seems to support exactly these two by default:
With indentation: {spaces: 4, check-multi-line-strings: true}
Turning on check-multi-line-strings results in 10K+ warnings, so no.
The other issue is the style ruamel.yaml wants to write out is as the patch does above. This matters when doing some scripted transformations where we read in the files and write them back out. I can somewhat work around that by first doing a pass with no changes and then another pass with the actual changes, but that's completely scriptable. Hopefully, ruamel learns to preserve the style better.
Kind of sad, but I understand the reason as far as my understanding of the yaml world allows. Thanks for the explanation.
[For idt,versaclock5.yaml, plus an overview of whole patch] Reviewed-by: Luca Ceresoli luca@lucaceresoli.net
participants (2)
-
Luca Ceresoli
-
Rob Herring