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

KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, remove "ccloud kafka topic mirror" command, and rename "--link-name" to "link" #906

Merged
merged 4 commits into from
Jun 30, 2021

Conversation

ctan888
Copy link
Contributor

@ctan888 ctan888 commented Jun 28, 2021

Checklist

  1. [CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?

    • yes: ok
    • no: DO NOT MERGE until the required functionalites are live in prod
  2. Did you add/update any commands that accept secrets as args/flags?

What

  1. Removing the cluster linking commands' kapi dependency from the ccloud cli
  2. Remove "ccloud kafka topic create" --mirror-topic and --link flags
  3. Remove "ccloud kafka topic mirror" command
  4. Rename "--link-name" to "link"

References

KGLOBAL-451 KGLOBAL-486

Test&Review

  1. Unit test by mocking the API payload
  2. Unit test by asserting the output (golden files)

@ctan888 ctan888 requested a review from a team as a code owner June 28, 2021 23:33
Copy link
Member

@brianstrauch brianstrauch left a comment

Choose a reason for hiding this comment

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

LGTM, although I assume there are a few tests that need to be fixed due to these deletions.

internal/cmd/kafka/command_topic.go Outdated Show resolved Hide resolved
@ctan888 ctan888 changed the title KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, and "ccloud kafka topic mirror" command KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, and "ccloud kafka topic mirror" command, rename "--link-name" to "link" Jun 29, 2021
Co-authored-by: Brian Strauch <bstrauch24@gmail.com>
@ctan888
Copy link
Contributor Author

ctan888 commented Jun 29, 2021

@brianbushree Yes, I've regenerated the golden files. Also, I've renamed all "--link-name" flag to "--link" according to @lukeknep 's suggestion. Let me know if we are good to go.

@ctan888 ctan888 changed the title KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, and "ccloud kafka topic mirror" command, rename "--link-name" to "link" KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, "ccloud kafka topic mirror" command, and rename "--link-name" to "link" Jun 29, 2021
@ctan888 ctan888 changed the title KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, "ccloud kafka topic mirror" command, and rename "--link-name" to "link" KGLOBAL-486: Remove "ccloud kafka topic create" --mirror-topic and --link flags, remove "ccloud kafka topic mirror" command, and rename "--link-name" to "link" Jun 29, 2021
Copy link
Contributor

@mtodzo mtodzo left a comment

Choose a reason for hiding this comment

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

We need to preserve backward compatibility with all CLI PRs. Changing flag names for link and mirror commands is ok since those are hidden (note that once we unhide them we don't be able to change these again without a 6 month deprecation process). Also the kafka topic mirror command is okay to remove since it has been hidden. However, we can't remove the the --mirror-topic and --link flags from topic create since those may be in use and we don't want to break existing scripts. We can remove those in CLI 2.0 release at the end of q3 since we will introduce breaking changes on the major release.

@scosta
Copy link
Member

scosta commented Jun 29, 2021

We can remove those in CLI 2.0 release at the end of q3 since we will introduce breaking changes on the major release.

If you haven't already, add your breaking changes to the v2.0 scheduled changes wiki to make sure this is tracked.

Copy link
Contributor

@mtodzo mtodzo left a comment

Choose a reason for hiding this comment

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

approved - discussed over slack that breaking change is OK since CL was only in preview

@ctan888 ctan888 merged commit 68821e4 into master Jun 30, 2021
@ctan888 ctan888 deleted the KILL_KAPI_CL branch June 30, 2021 18:35
brianstrauch pushed a commit that referenced this pull request Jun 30, 2021
…link flags, remove "ccloud kafka topic mirror" command, and rename "--link-name" to "link" (#906)
brianstrauch pushed a commit that referenced this pull request Jul 1, 2021
…link flags, remove "ccloud kafka topic mirror" command, and rename "--link-name" to "link" (#906)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants