Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

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 No newline at end of file

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants