-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
cmd/operator-sdk/run: remove legacy run subcommand #3406
cmd/operator-sdk/run: remove legacy run subcommand #3406
Conversation
We have some links in the CHANGELOG referencing We could update those to be non-relative links https://v0-18-x.sdk.operatorframework.io/docs/cli/operator-sdk_run/ Or just use the git sha if we don't want to point to the website in such an older release. |
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.
You can also remove this check and these lines. Also it looks like a reference to the removed CLI doc for run
still exists somewhere.
Ah this gets rid of |
1cf804e
to
b8c19f4
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.
/lgtm
@hasbro17 🤣 Sorry, I got overzealous. I updated the links to point to the docs as they existed at the time when they were referenced (so to our old github docs at the v0.15.0 tag) |
I also went ahead and removed legacy cleanup command support with the same changes that @estroz suggested. |
@estroz We can remove the following internal pkgs/code for legacy But I can do a follow up for that. |
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.
LGTM
Per offline discussion, this needs a CHANGELOG fragment so we can track and summarize all CLI changes in the |
If CI passes on this commit, let's just merge and I'll do a fragment follow-up. I need to do one for #3385 as well. |
New changes are detected. LGTM label has been removed. |
e2201f5
to
9149775
Compare
# release notes and/or the migration guide | ||
entries: | ||
- description: > | ||
Remove legacy run amd cleanup commands |
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.
amd -> and
Description of the change:
Remove the legacy
run
subcommand. This also incidentally removes support forrun local
for legacy Ansible and Helm projectsMotivation for the change:
To remove legacy code in preparation for 1.0 release.
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs