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

refactor: renamed delete commands #3056

Merged
merged 5 commits into from
Jul 21, 2022
Merged

refactor: renamed delete commands #3056

merged 5 commits into from
Jul 21, 2022

Conversation

vvagaytsev
Copy link
Collaborator

@vvagaytsev vvagaytsev commented Jul 20, 2022

What this PR does / why we need it:
This PR only contains the user-facing parts for now. We can rename internals later. The internal includes the classes like DeleteXxx[Command|Params|Result], some handlers, methods and functions in the action router, local variables, etc.

Changing the internal now can cause extra conflicts with https://github.com/garden-io/garden/tree/graph-v2.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@vvagaytsev vvagaytsev changed the base branch from master to 0.13 July 20, 2022 11:40
@vvagaytsev vvagaytsev force-pushed the rename-delete-commands branch from 6cec321 to 828c520 Compare July 20, 2022 14:58
@vvagaytsev vvagaytsev force-pushed the rename-delete-commands branch from 828c520 to a1e39df Compare July 20, 2022 15:31
@vvagaytsev vvagaytsev changed the title Rename delete commands feat: renamed delete commands Jul 20, 2022
@vvagaytsev vvagaytsev requested review from edvald and Orzelius July 20, 2022 15:57
@vvagaytsev vvagaytsev marked this pull request as ready for review July 20, 2022 15:57
@@ -24,9 +24,8 @@ import { LogEntry } from "../logger/log-entry"
import { uniqByName } from "../util/util"

export class DeleteCommand extends CommandGroup {
name = "delete"
alias = "del"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd suggest keeping delete as an alias.

alias = "env"
help = "Deletes a running environment."
name = "namespace"
alias = "ns"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe throw a TODO-G2 comment in here. I had already made a plural aliases field in the graph-v2 branch, and it'd be good to have environment and env in there as aliases as well.

@vvagaytsev vvagaytsev requested a review from edvald July 21, 2022 07:55
@vvagaytsev vvagaytsev changed the title feat: renamed delete commands refactor: renamed delete commands Jul 21, 2022
Copy link
Contributor

@Orzelius Orzelius left a comment

Choose a reason for hiding this comment

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

Looks good! I guess the renaming of the files will be done afterwards as well

@vvagaytsev
Copy link
Collaborator Author

@Orzelius thank you! Yes, we'll rename the files later.

@vvagaytsev vvagaytsev merged commit d4b9986 into 0.13 Jul 21, 2022
@vvagaytsev vvagaytsev deleted the rename-delete-commands branch July 21, 2022 15:42
This was referenced May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants