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

vdk-control-cli: Fix command printed on successful deploy #839

Merged
merged 2 commits into from
May 19, 2022

Conversation

gabrielgeorgiev1
Copy link
Contributor

@gabrielgeorgiev1 gabrielgeorgiev1 commented May 19, 2022

When successfully deploying a job, vdk-control-cli
prints a command which can be used to check whether
the deployment version was successfully updated.
However, the printed command included the team name
without quotes around it, which can be an issue as
team names can have empty spaces in them, which meant
the command gets parsed wrongly and fails.

Command used to look like:

vdk deploy --show -t team_name -n job_name

Now it looks like:

vdk deploy --show -t 'team_name' -n 'job_name'

Signed-off-by: Gabriel Georgiev [email protected]

When successfully deploying a job, vdk-control-cli
prints a command which can be used to check whether
the deployment version was successfully updated.
However, the printed command included the team name
without brackets around it, which can be an issue as
team names can have empty spaces in them, which meant
the command gets parsed wrongly and fails.

Signed-off-by: Gabriel Georgiev <[email protected]>
Copy link
Contributor

@doks5 doks5 left a comment

Choose a reason for hiding this comment

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

In the description of the PR, it is said that the message is missing brackets. Should it actually say quotes?

@gabrielgeorgiev1 gabrielgeorgiev1 enabled auto-merge (squash) May 19, 2022 08:07
@gabrielgeorgiev1 gabrielgeorgiev1 merged commit 591ddc5 into main May 19, 2022
@gabrielgeorgiev1 gabrielgeorgiev1 deleted the person/gageorgiev/fix-deploy-check-msg branch May 19, 2022 09:25
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