Internal go-yaml/yaml fork #4013
Conversation
|
Skipping CI for Draft Pull Request. |
|
@KnVerey: This PR has multiple commits, and the default merge method is: merge. 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/test-infra repository. |
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: KnVerey The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
cncf-cla: yes
kyaml/internal/forked/README.md
Outdated
|
|
||
| ## go-yaml/yaml | ||
|
|
||
| This code is used extensively by kyaml. It is a copy of upstream at a particular revision that kubectl is using, with [a change we need](https://github.com/go-yaml/yaml/pull/746) cherry-picked on top. For context on why the cherry-pick to an old revision is necessary, see https://github.com/kubernetes-sigs/kustomize/issues/3946. |
There was a problem hiding this comment.
Note to self: update this last sentence if we use this--it's not to an old revision anymore, and the pull request reference is wrong
k8s-ci-robot
added
size/XXL
|
/test kustomize-presubmit-master |
1 similar comment
|
/test kustomize-presubmit-master |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
KnVerey
changed the title
|
/lgtm |
k8s-ci-robot
added
the
lgtm
| @@ -33,7 +43,10 @@ var Unmarshal = yaml.Unmarshal | |||
| var NewDecoder = yaml.NewDecoder | |||
| var NewEncoder = func(w io.Writer) *yaml.Encoder { | |||
| e := yaml.NewEncoder(w) | |||
| e.SetIndent(2) | |||
| e.SetIndent(indent) | |||
| if sequenceIndentationStyle == CompactSequenceStyle { | |||
There was a problem hiding this comment.
@KnVerey @natasha41575 What is the reason behind not making this configurable ? Will it be exposed in the future ?
There was a problem hiding this comment.
kpt doesn't hold strong opinion about seq indentation, but we want minimum disruption to the users. The plan is to identify the existing indentation of package resources and retain it, for which, we need this to be configurable. Let me know your thoughts.
As of now, kpt can't upgrade to latest kyaml v0.11.0
@natasha41575 here's a script we can use to make an internal copy of yaml.v3 with your changes applied. It pulls down the latest version from upstream and then applies your commit instead of pulling your branch directly. That's not strictly necessary now, since your changes are based on the same commit k/k is using. But if we need to merge this, we could end up in a situation where your changes need to be rebased to a version that is ahead of k/k (as was the case when Jeff was working on his version of the changes).
To use this:
CHERRY_PICK_SHAif necessary (e.g. if the go-yaml PR changes)ALLOW_MODULE_SPAN