-
Notifications
You must be signed in to change notification settings - Fork 2
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
Remove unused command code #2569
base: main
Are you sure you want to change the base?
Conversation
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.
This is great!
Approving with some nits, seems also the errTlsRequiredFiles
error can be removed now.
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.
Once we get CI passing this is looking good. Agreed with @marcosnav, lets get those comments cleaned up 🔥
To get CI to pass, we have to merge #2584 first. That PR is waiting on replacement tests for what it is removing. |
Intent
Removes legacy CLI commands which are not needed for VSCode extension
Resolves #2521
Type of Change
Approach
The VSCode extension executes the agent with the following command parameters:
<agent> UI -vv --listen <port>
It is also useful to support
<agent> version
I removed the CLI commands within the
kong
infrastructure and then trimmed down the sub-commands locatedwithin the UI command. There were a few impacts to the httpserver that we are setting up, but that was for features which were not in use.
User Impact
The user should see no difference other than when executing
<agent> -h
(help).Automated Tests
Bat tests have been removed, which were what was executing the CLI. This was done in a different PR (#2558)
Directions for Reviewers
Build checks and CI should pass. Manual tests should be performed to just make sure the UX is able to communicate with the agent.
Checklist