Skip to content

[gp-cli] Move related subcommands to ports cmd #10462

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

Closed
wants to merge 1 commit into from
Closed

[gp-cli] Move related subcommands to ports cmd #10462

wants to merge 1 commit into from

Conversation

CuriousCorrelation
Copy link
Contributor

Description

This PR moves ports related commands to portsCmd

Related Issue(s)

#10449

How to test

gp forward-port and gp await-port are now gp ports forward and gp ports await and work as expected.

Documentation

Does this PR require updates to the documentation at www.gitpod.io/docs?

@CuriousCorrelation CuriousCorrelation requested review from a team June 3, 2022 16:24
@roboquat
Copy link
Contributor

roboquat commented Jun 3, 2022

@CuriousCorrelation: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@CuriousCorrelation
Copy link
Contributor Author

I have also refactored files in components/ws-manager/pkg/manager/testdata/
gp await-port to gp ports await.
I hope that falls in the scope, if not I can revert them in my next commit.

Please let me know if there's something I missed.
I'll create an issue in docs once this gets green light!

@iQQBot
Copy link
Contributor

iQQBot commented Jun 3, 2022

Hi @CuriousCorrelation, thanks for your contribution

I would like to ask why this change was made? This is a breaking change, many users already use commands like gp await-port in their repo and if this PR is merged, these users will not be able to use

@iQQBot
Copy link
Contributor

iQQBot commented Jun 3, 2022

Okay, I checked the original issue and what we should do is add the alias and not break the original command

@CuriousCorrelation
Copy link
Contributor Author

Ah I understand. I'll close this PR and make the changes and resubmit. Thanks for the info!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants