added warning message for deprecated flag #193
Conversation
Pull Request Test Coverage Report for Build 2829268954
💛 - Coveralls |
| @@ -119,6 +119,7 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { | |||
| fs.StringVar(&p.options.chartOptions.Version, helmChartVersionFlag, "", "helm chart version (default: latest)") | |||
|
|
|||
| fs.StringVar(&p.options.CRDVersion, crdVersionFlag, defaultCrdVersion, "crd version to generate") | |||
| _ = fs.MarkDeprecated(crdVersionFlag, util.WarnMessageRemovalV1beta1) | |||
There was a problem hiding this comment.
Note that to work with v1beta1 we need old versions of controller-gen and controller-runtime deps so I do not think that this option can work well. Therefore, I think that we could also remove them since the plugin is alpha.
There was a problem hiding this comment.
@varshaprasad96 wdyt?
There was a problem hiding this comment.
Agreed @camilamacedo86. But does the helm plugin in SDK has removed it? We need to be consistent with the changes happening in SDK's Helm and the one here.
There was a problem hiding this comment.
Looks like its not (https://github.com/operator-framework/operator-sdk/blob/fd893783327b68fbbdd118cbc0b4e1b6d4370b4c/internal/plugins/helm/v1/api.go#L128). Can we remove it there first?
There was a problem hiding this comment.
@varshaprasad96 the helm one is v1 so we could not remove but that is probably not working there without the user downgrading the controller tools used manually.
Here is a little worst because that would mean changing the go.mod either to downgrade the controller runtime as well.
However, anyway, if we want here to deprecate for now and remove afterwords I am ok with too 👍
There was a problem hiding this comment.
thank you @camilamacedo86 and @varshaprasad96. This PR got two approvals so I am merging it now.
This will fix #185
I added a warning message for the deprecated flag. PLease take a look once.