refactor: Remove setters from kyaml #5291
Conversation
Kind: Refactor Summary: Setters functionality is provided as a KRM function. We should remove code related to setters in cmd/config and kyaml. As of now most setters2 and setters usage are related to fork of kpt, however, these: [fluxcd/image-automation-controller](https://github.com/fluxcd/image-automation-controller/blob/6827808a1aa8c31ae0c05366db8e90c011f6ebd2/pkg/update/filter.go#L24) with [kyml](https://github.com/fluxcd/image-automation-controller/blob/6827808a1aa8c31ae0c05366db8e90c011f6ebd2/go.mod#L42) [rancher/fleet](https://github.com/rancher/fleet/blob/0a6cf6cb9278d00e743e79ed1884b19e1bc6ec74/internal/cmd/controller/controllers/image/update/setters.go#L16) with [kyaml](https://github.com/rancher/fleet/blob/0a6cf6cb9278d00e743e79ed1884b19e1bc6ec74/go.mod#L75) Repositories still using them, They pinned these two into a specific kyaml version. If we decide to go for this removal then we can make a release note that this is actually removed on the next version since we already marked this as deprecated before.
k8s-ci-robot
added
cncf-cla: yes
|
/assign |
|
A note on assignments, assignments on PRs are for reviewers, not for yourself. Please ask one of the reviewers in your area to see if any of them can assign this PR to themselves for review. I would recommend asking on slack. |
|
@natasha41575 this should be you isn't it? at leas based on the owner list here |
The process we're following is that you first need an LGTM from folks in the reviewer list, and I'll take a look when after that. That way more people get a chance to review PRs |
|
/assign seh |
|
@irvifa: GitHub didn't allow me to assign the following users: seh. Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
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. |
|
@irvifa
So, Where do you find deprecated messages? |
Thanks! I checked that. @natasha41575 /lgtm |
k8s-ci-robot
added
the
lgtm
The top-level kustomize commands related to setters were marked deprecated and removed, but this package that we are removing now was never marked deprecated.
@koba1t No particular reason. I think we just didn't think about it. But I agree, this is mostly an internal-use package and kyaml is in alpha so there is no obligation for us to go through an official deprecation process. Thanks for the review and LGTM, I'm moving this to Needs Approval and will try to take a look next week. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: irvifa, natasha41575 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
The package setters2 has been removed from kustomize: kubernetes-sigs/kustomize#5291 This commit removes the need to import setters2, by reproducing the last link with that code, which is a minor parsing helper. I have not changed the comment explaining what was changed from the original, since it's still accurate. The parsing func is a buried detail. Signed-off-by: Michael Bridgen <mikeb@squaremobius.net>
The package setters2 has been removed from kustomize: kubernetes-sigs/kustomize#5291 This commit removes the need to import setters2, by reproducing the last link with that code, which is a minor parsing helper. I have not changed the comment explaining what was changed from the original, since it's still accurate. The parsing func is a buried detail. Signed-off-by: Michael Bridgen <mikeb@squaremobius.net>
The package setters2 has been removed from kustomize: kubernetes-sigs/kustomize#5291 This commit removes the need to import setters2, by reproducing the last link with that code, which is a minor parsing helper. I have not changed the comment explaining what was changed from the original, since it's still accurate. The parsing func is a buried detail. Signed-off-by: Michael Bridgen <mikeb@squaremobius.net>
Kind:
Refactor
Summary:
Setters functionality is provided as a KRM function. We should remove code related to setters in cmd/config and kyaml. As of now most setters2 and setters usage are related to fork of kpt, however, these: fluxcd/image-automation-controller with kyml rancher/fleet with kyaml
Repositories still using them, They pinned these two into a specific kyaml version. If we decide to go for this removal then we can make a release note that this is actually removed on the next version since we already marked this as deprecated before.
Closes #4045