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

feat: remove update command which was useful just for v1 #1489

Merged
merged 1 commit into from
May 8, 2020

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented May 3, 2020

Description

We no longer need to keep this command to help to migrate the projects to use go mod.
It was useful just to v1 projects which were deprecated to long ago and is no longer supported.

Motivation
Suggestions/comments raised when we remove the v1 code source.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 3, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from droot and mengqiy May 3, 2020 13:14
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 3, 2020
@camilamacedo86
Copy link
Member Author

c/c @estroz

@estroz
Copy link
Contributor

estroz commented May 4, 2020

@mengqiy @droot do you foresee this command being used in the future? Perhaps it would be better to remove the Gopkg scaffold, leave the command constructor, and only add this command to the CLI after checking updateCmd.HasSubCommands() so we can use update later.

@camilamacedo86
Copy link
Member Author

HI @estroz,

Really tks for your input and help with.

@mengqiy @droot do you foresee this command being used in the future? Perhaps it would be better to remove the Gopkg scaffold, leave the command constructor, and only add this command to the CLI after checking updateCmd.HasSubCommands() so we can use update later.

As we spoke, I do not think that we should keep the command at all. See that it is just useful for V1 projects and then, I am afraid that by following your suggestion we will keep a command that has no utility with the justification that we might need or not an updated cmd that works in another way in the future. So, IHMO it shows quite safe be removed.

However, let's see if @mengqiy and/or @droot see any reason for we keep it.

@estroz
Copy link
Contributor

estroz commented May 8, 2020

It would be trivial to add back, ex. git checkout v2.3.0 -- cmd/update.go, so we can get rid of it here.

@estroz
Copy link
Contributor

estroz commented May 8, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2020
@k8s-ci-robot k8s-ci-robot merged commit 1532fb6 into kubernetes-sigs:master May 8, 2020
@camilamacedo86 camilamacedo86 deleted the remove-update branch May 8, 2020 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants