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

Deprecate some cfg commands #4048

Merged
merged 1 commit into from Jul 22, 2021

Conversation

natasha41575
Copy link
Contributor

@natasha41575 natasha41575 commented Jul 8, 2021

This PR marks some cfg commands as deprecated as described in #3953, and would be a first step toward #4045.

Some questions:

  1. Can we do this now? There is some discussion in the Deprecation of kustomize cfg and kustomize fn alpha commands #3953 about having setters as a transformer - do we need to wait for that to be done before deprecating the setters commands? Or is a release note pointing to the KRM functions enough?
  2. I haven't seen any discussion about the command create-subst - this PR marks it as deprecated, but is that what we want to do?
  3. Some of the wording for the deprecation messages can probably be improved, I would appreciate some feedback for that.

Sample output:

$ kustomize cfg annotate
Command "annotate" is deprecated, use `commonAnnotations` field in kustomization instead.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 8, 2021
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 8, 2021
@KnVerey
Copy link
Contributor

KnVerey commented Jul 15, 2021

Can we do this now? There is some discussion in the [Question] Future of kustomize cfg alpha commands #3953 about having setters as a transformer - do we need to wait for that to be done before deprecating the setters commands? Or is a release note pointing to the KRM functions enough?

IMO yes we can do it now. In fact, it might get us some engagement around whether it is worth providing as an "official" transformer.

I haven't seen any discussion about the command create-subst - this PR marks it as deprecated, but is that what we want to do?

Looks like it's related to setters and requires a KRM file, so yes, I think it should go.

Some of the wording for the deprecation messages can probably be improved, I would appreciate some feedback for that.

WDYT about linking to the deprecation issue?

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 19, 2021
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 19, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from k8s-ci-robot Jul 19, 2021
@natasha41575 natasha41575 force-pushed the DeprecateCfgCmds branch 2 times, most recently from b725807 to f95b409 Compare July 19, 2021 21:26
@natasha41575
Copy link
Contributor Author

natasha41575 commented Jul 19, 2021

WDYT about linking to the deprecation issue?

@KnVerey done, e.g.

$ kustomize cfg create-setter <args>
Command "create-setter" is deprecated, setter commands will no longer be available in kustomize v5.
See discussion in https://github.com/kubernetes-sigs/kustomize/issues/3953.

Copy link
Contributor

@KnVerey KnVerey left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 22, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KnVerey, 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:
  • OWNERS [KnVerey,natasha41575]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 94c5096 into kubernetes-sigs:master Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants