-
Notifications
You must be signed in to change notification settings - Fork 217
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
kustomize: add deprecation warning to docs #289
kustomize: add deprecation warning to docs #289
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
/remove-lifecycle rotten |
I need to update this PR to reflect all the deprecated fields, and change the version number. |
@natasha41575 can you comment on when these features will be removed from kustomize? Is there an issue or milestone for their planned removal? Looking to understand what our runway is to remove these from our code base. Thank you! |
412a4db
to
5c72fd9
Compare
@holdeneoneal Thank you for the very valid question! In response to your question, I added the answer to the docs in this PR so that it is really clear what the timeline is, which applies to all deprecated fields:
There is no strict timeline for this, but realistically the removal of these fields will likely take at least a year or two. It is only when we have deprecated Kustomization v1beta1 entirely that we will be in a position to drop support for deprecated fields completely, so you should keep an eye out for that announcement. |
76a0b53
to
38f16e7
Compare
@KnVerey This is ready for review! |
/cc @koba1t @annasong20 |
/hold So that this isn't merged until release |
Thank you, Natasha! I have a follow up question. When you say "at least two releases", are those minor version releases of kustomize or major version releases? |
Any two releases, I think this includes patch/minor releases. Kustomize is also built into kubectl, so we generally have to follow their deprecation policy: https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli |
Thanks, @natasha41575! So, I think the legacy patches syntax is removed next release. |
site/content/en/references/kustomize/kustomization/bases/_index.md
Outdated
Show resolved
Hide resolved
Great point! That syntax is super old and currently nowhere in the docs, but we added its removal to the release notes. |
38f16e7
to
cfa5509
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple copy-paste things to clean up. Should we mention kustomize edit fix
for any of these?
@@ -7,6 +7,19 @@ description: > | |||
Patch resources using the strategic merge patch standard. | |||
--- | |||
|
|||
{{% pageinfo color="warning" %}} | |||
The `patchesStrategicMerge` field was deprecated in v5.0.0. The `bases` field was deprecated in v2.1.0. This field will never be removed from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `patchesStrategicMerge` field was deprecated in v5.0.0. The `bases` field was deprecated in v2.1.0. This field will never be removed from the | |
The `patchesStrategicMerge` field was deprecated in v5.0.0. This field will never be removed from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♀️ Sorry about that, I should be proof reading my own PRs...
site/content/en/references/kustomize/kustomization/patchesjson6902/_index.md
Outdated
Show resolved
Hide resolved
site/content/en/references/kustomize/kustomization/vars/_index.md
Outdated
Show resolved
Hide resolved
cfa5509
to
9d1f803
Compare
Done! And added a warning that the automatic vars->replacement conversion may fail. |
/unhold |
/lgtm |
Should be merged after release containing the deprecation warnings
/hold
/cc @KnVerey