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

added a flag to indent or not indent the sequence #750

Closed
wants to merge 2 commits into from

Conversation

mikebz
Copy link

@mikebz mikebz commented Jun 17, 2021

fixes #720

This provides a function where one can set whether to indent or not indent the sequence. Not indenting the sequence provides a smoother upgrade path from v2 to v3. A lot of tools (like kustomize) would break their clients if the expected yaml differed.

If we expect more of these types of output configurations we should consider creating a formatting flags structure. Right now we just have the indent size and this flag, but maybe it will grow in the future. Happy to do either one, but my guess is that right now this is a stop gap for updates.

Review needed

  1. @niemeyer - does this align with your approach?
  2. Should we add more tests? Any cases that are crucial to cover?

@@ -0,0 +1 @@
.vscode

Choose a reason for hiding this comment

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

why add this?

Copy link
Author

Choose a reason for hiding this comment

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

VS Code is a popular editor. I was surprised that there was not a gitignore for this.

func (s *S) TestSetIndentSequences(c *C) {
var buf bytes.Buffer
enc := yaml.NewEncoder(&buf)
enc.SetIndentSequences(false)

Choose a reason for hiding this comment

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

should there be a test for setting it to true?

Copy link
Author

Choose a reason for hiding this comment

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

yes, it's actually true (well the inverse under the hood) by default so the above tests for sequences cover it. We can add an explicit set to true and make sure it works. It's not a bad idea.

@mikebz
Copy link
Author

mikebz commented Jun 22, 2021

Superceded by @natasha41575 's #753

@mikebz mikebz closed this Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants