wip: lint all yaml files using sigs.k8s.io/yaml/yamlfmt #36186
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
size/XL
…mt, updating dependencies and build scripts.
… on the `ci` variable.
…, env:) to be indented 2 spaces relative to their parent key. The original template sometimes had them at the same level or aligned incorrectly. We shifted them to match the standard.
- Removing "Invisible" Newlines: Loop and Option tags in Jinja (like {% if %}) leave behind blank lines by default. We changed them to {%- if -%} (adding hyphens) to tell Jinja: "Execute this logic, but strip the whitespace around it." This stopped valid code from being pushed apart by random blank lines.
- Stripping the End: We added {%- endfilter -%} at the very end. This removes all trailing newlines from the template output itself, giving the Python script full control over how jobs are separated.
…red, we ensure that: There is exactly one newline between jobs (separating them cleanly). There is exactly one newline at the end of the file (making it a valid text file).
medyagh
force-pushed
the
yamllinter_all
branch
from
dff9e8a to
b72f839
Compare
k8s-ci-robot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: medyagh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/XL
|
@medyagh: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
This PR doesn't touch anything related to sig multicluster, and the k/k Makefile and hack scripts also doesn't affect any sig multicluster projects because they are all out of tree. /remove-label sig-multicluster |
|
@lauralorenz: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/remove-sig multicluster |
k8s-ci-robot
removed
the
sig/multicluster
It must be touching a directory with OWNERS configured to label sig-multi-cluster, if SIG multi-cluster doesn't own anything here, please send a PR to remove this configuration. The OWNERS-based label application is evaluated on every push, because the set of files can change on every push. I think in this case, the issue is a previous iteration had applied auto-formatting to all yaml config files, which would've touched e.g. this folder: Sorry, we still need additional reviews on this one, cc @pohly for dra.jinja generate-jobs.py LGTM, and I think conceptual +1 overall from myself, @aojea @upodroid posting to #sig-testing |
|
+1 from me |
| {%- if run_if_changed and presubmit %} | ||
| run_if_changed: {{run_if_changed}} | ||
| {%- endif -%} | ||
| {%- set indent_width = 4 if not ci else 2 -%} |
There was a problem hiding this comment.
That indention is different between periodics and presubmits is annoying also when copy-and-pasting between them manually, but that ship has sailed a long time ago...
Thanks for coming up with a solution.
| run_if_changed: {{run_if_changed}} | ||
| {%- endif -%} | ||
| {%- set indent_width = 4 if not ci else 2 -%} | ||
| {%- filter indent(width=indent_width, first=True) -%} |
There was a problem hiding this comment.
Is the removal of the empty line before each job intentional because yamlfmt complains?
I find the YAML a bit easier to read when different jobs are separated a bit more. If we can have the empty line, then let's keep it. With this patch there is one more empty line after the header, but that is okay and perhaps even better because of consistency:
diff --git a/config/jobs/kubernetes/sig-node/dra.jinja b/config/jobs/kubernetes/sig-node/dra.jinja
index ca7244aa98..13aa089add 100644
--- a/config/jobs/kubernetes/sig-node/dra.jinja
+++ b/config/jobs/kubernetes/sig-node/dra.jinja
@@ -7,7 +7,7 @@ presubmits:
kubernetes/kubernetes:
{%- endif %}
{%- endif %}
-{% if not ci %}
+{%- if not ci %}
{%- set testgrid_dashboards = testgrid_dashboards + ", sig-node-presubmits" %}
{%- endif -%}
{%- if "crio" in job_name %}
@@ -22,8 +22,9 @@ presubmits:
{%- set testgrid_dashboards = testgrid_dashboards + ", sig-release-master-informing" %}
{%- set testgrid_alert_email = testgrid_alert_email + ", release-team@kubernetes.io" %}
{%- endif -%}
-{%- set indent_width = 4 if not ci else 2 -%}
-{%- filter indent(width=indent_width, first=True) -%}
+{%- set indent_width = 2 if ci else 4 -%}
+{% filter indent(width=indent_width, first=True) %}
+
- name: {{job_name}}
cluster: {{cluster}}
{%- if ci %}
diff --git a/hack/generate-jobs.py b/hack/generate-jobs.py
index 2e7ee98847..55d4767290 100755
--- a/hack/generate-jobs.py
+++ b/hack/generate-jobs.py
@@ -99,7 +99,6 @@ def generate_one(path: pathlib.Path, verify: bool) -> typing.List[str]:
)
)
header = ""
- tmp.write("\n")
out = template_path.parent / f"{template_path.stem}-{name}.yaml"
if not os.path.exists(out):
if verify:7 participants
This PR could be reviewed after this #36183
with all yaml files linted with the new Linter. I made two PRs for easier review