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

Consistent YAML formatting #1722

Closed
frankfarzan opened this issue Apr 8, 2021 · 8 comments · Fixed by #1813
Closed

Consistent YAML formatting #1722

frankfarzan opened this issue Apr 8, 2021 · 8 comments · Fixed by #1813
Assignees
Labels
area/hydrate area/pkg p1 size/L 4 day triaged Issue has been triaged by adding an `area/` label
Milestone

Comments

@frankfarzan
Copy link
Contributor

frankfarzan commented Apr 8, 2021

kpt needs to consistently format resources. Previously, there was kpt cfg fmt which is now removed in favor of having a global flag that controls styling/formatting as a cross-cutting feature.

As part of this, all examples in /package-examples (highest priority) and test data should be pre-formatted (Unless testing formatting logic itself). For example, eval comand on the nginx package in the Quickstart produces formatting changes which creates unnecessary noise:

https://kpt.dev/book/01-getting-started/02-quickstart

What's worse, there's no way to automatically preformat the example. We should also reconsider removing fmt command.

@frankfarzan
Copy link
Contributor Author

cc @phanimarupaka @droot @mortent

@droot
Copy link
Contributor

droot commented Apr 8, 2021

I think having a dedicated command to format resources sounds useful to me. Which subcommand group we should bring it in ?

@frankfarzan
Copy link
Contributor Author

frankfarzan commented Apr 8, 2021

kpt pkg format is consistent with kpt pkg view (fka tree), etc. In english: "I want to format my package".

@phanimarupaka
Copy link
Contributor

phanimarupaka commented Apr 8, 2021

How about having it as a format function instead ? By default any such functionality should be a function unless there is a great need to include it as part of cli. kpt pkg tree/kpt pkg view/kpt pkg desc qualifies to be a cli command as will be frequently used and is the first thing a consumer would do.

@mikebz mikebz added the triaged Issue has been triaged by adding an `area/` label label Apr 12, 2021
@phanimarupaka phanimarupaka linked a pull request Apr 25, 2021 that will close this issue
@phanimarupaka
Copy link
Contributor

@mengqiy
Copy link
Contributor

mengqiy commented May 5, 2021

It seems we start to format the resources in kpt fn render (#1813).
@phanimarupaka Is there a reason not doing it for kpt fn eval?

@phanimarupaka
Copy link
Contributor

Yes. eval can be invoked on validator functions as well. Mutating resources in such scenarios looks awkward. If users want to format resources, they can invoke explicit format image.

@mengqiy
Copy link
Contributor

mengqiy commented May 5, 2021

Thanks for clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hydrate area/pkg p1 size/L 4 day triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants