deprecate git-init flag #3241
deprecate git-init flag #3241
Conversation
| @@ -389,6 +394,8 @@ func verifyFlags() error { | |||
| return nil | |||
| } | |||
|
|
|||
| // Deprecated: the git-init flag was deprecated since has no need to make the command run the git init. | |||
There was a problem hiding this comment.
you could in addition to adding the deprecation move it to another file, so when the time comes you just need to delete the file and make the code build again
There was a problem hiding this comment.
Tks for the suggestion. But in this case, I do not think that it is required/worth to do that.
cmd/operator-sdk/new/cmd.go
Outdated
| newCmd.Flags().BoolVar(&gitInit, "git-init", false, | ||
| "Initialize the project directory as a git repository (default false)") | ||
| _ = newCmd.Flags().MarkDeprecated("git-init", "This flag will be removed soon. "+ |
There was a problem hiding this comment.
you can also extract that to an external file I think, just pass the newcmd as an argument
| @@ -190,7 +195,7 @@ func newFunc(cmd *cobra.Command, args []string) error { | |||
| log.Fatal(err) | |||
| } | |||
| } | |||
|
|
|||
| //todo: remove before 1.0.0 | |||
| if gitInit { | |||
| if err := initGit(); err != nil { | |||
| log.Fatal(err) | |||
|
I do not know your convension, but should the commits be squashed to one big commit? if not I will add a /lgtm on my side |
cmd/operator-sdk/new/cmd.go
Outdated
| newCmd.Flags().BoolVar(&gitInit, "git-init", false, | ||
| "Initialize the project directory as a git repository (default false)") | ||
| _ = newCmd.Flags().MarkDeprecated("git-init", "This flag will be removed soon. "+ |
There was a problem hiding this comment.
Check this error with a panic, since this should never occur.
There was a problem hiding this comment.
Should we really give a panic if the flag was not marked as deprecated?
IMO shows fine in this case just suppress the error.
There was a problem hiding this comment.
Right now the flag output is
Flag --git-init has been deprecated, This flag will be removed soon. To continue using git, run `git init` once your project is created.
The following reads better:
Flag --git-init has been deprecated, instead run `git init` once your project is created to use git
So change the help text to:
| _ = newCmd.Flags().MarkDeprecated("git-init", "This flag will be removed soon. "+ | |
| _ = newCmd.Flags().MarkDeprecated("git-init", "instead run `git init` once your project is created to use git") |
There was a problem hiding this comment.
@camilamacedo86 MarkDeprecated will only error if the flag name is not found, which it always will be. If it isn't the SDK should panic, like in other commands.
There was a problem hiding this comment.
which it always will be . Exactly, because of this shows not a scenario that really needs to be covered. However, it is fine. The suggestion was applied. Tks for the review.
|
Hi @joelanford, Are you ok with move with it now? |
openshift-ci-robot
added
the
needs-rebase
openshift-ci-robot
removed
lgtm
Description of the change:
deprecate git-init flag in the new cmd
Motivation for the change:
Has no need for we have a flag for it. Users are able to run git init when they wish.