Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce a standard YAML format #763

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stuartwdouglas
Copy link
Contributor

This PR requires all files to be formatted with the google YAML formatter before submitting. This can be done with the following commands:

go install github.com/google/yamlfmt/cmd/[email protected] yamlfmt .

This will enforce a standard style that should remove any issues with formatting of PRs.

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

This PR requires all files to be formatted with the google YAML formatter before submitting. This can be done with the following commands:

go install github.com/google/yamlfmt/cmd/[email protected] yamlfmt .

This will enforce a standard style that should remove any issues with formatting of PRs.

The actual formatting is done in a subsequent commit.
Copy link

sonarcloud bot commented Jan 30, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@stuartwdouglas stuartwdouglas changed the title (DO NOT MERGE) Enforce a standard YAML format Enforce a standard YAML format Jan 30, 2024
@prietyc123
Copy link

@stuartwdouglas Is there any specific reason you are using v0.10.0 yamlfmt. Can't be do it based on Red Hat YAML lint?
One of the ex: With the current yaml format I see this approves the checks even if the yamls not started with

---

@prietyc123
Copy link

Asking this because I am working on enabling the yamllint for tssc-sample-pipelines repo and I have these rules . Since the sample-pipelines repo sync some yamls from this repository, I want to understand this a bit in depth.

@@ -0,0 +1,28 @@
name: Validate PR - YAML Format

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't It would be best if the file started with ---

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --- makes file structure more explicit, clearer and readable, especially when dealing with complex yaml documents that contains multiple documents concatenated together. I don't think yamlfmt handles this. Correct me if I am wrong.

@stuartwdouglas
Copy link
Contributor Author

Doesn't yaml lint just check formatting? Or can it automatically fix the formatting as well?

The idea of this is that there is a single standard format, and you can just run the formatter and not have to think about formatting ever again. It means you won't get PRs with formatting changes, and all the files will be consistent.

@prietyc123
Copy link

Doesn't yaml lint just check formatting? Or can it automatically fix the formatting as well?

yes yamllint just check formatting but my point is the formatting that yamlfmt is doing might be lacking few of the standard like file should start with --- or if some values started with block but end with scalar then it should wrap it inside the quotes spack/spack#38408 etc.
How would you fix those? Manually?

@prietyc123
Copy link

ok maybe for now I will remove the document-start: enable from the sample pipeline repo yamllint check and this fix should help the syncing linter issue. So lets merge this if everything else is good

/lgtm

@chmeliik
Copy link
Contributor

Looks abandoned, can we close this?

@openshift-merge-robot
Copy link

PR needs rebase.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants