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

Remove deprecated options & set default value for ec2 ami list owner option #586

Merged

Conversation

kvivek1115
Copy link
Contributor

Description

  • Removed options --tag-node-in-chef and --security-group-ids
  • As --tags also deprecated but available in chef-core so --tags removed from knife-ec2.
  • Currently --tags and --chef-tag do the same things so we can also deprecated --chef-tag from knife-ec2 plugin?

Check List

Copy link
Contributor

@dheerajd-msys dheerajd-msys left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@btm
Copy link
Contributor

btm commented Jun 5, 2019

we can deprecate --chef-tag but in the message we need to note that currently --tags in core takes this format: --tags foo,bar,zap while we had moved --chef-tag to use this format: --chef-tag foo --chef-tag bar --chef-tag zap...

at some point we should switch code to use the other pattern, but for now the deprecation message should point out the difference.

@kvivek1115
Copy link
Contributor Author

Hey @btm Thanks for reviews comment I have deprecated --chef-tag and added deprecation warning [DEPRECATED] --chef-tag option is deprecated and will be removed in future release. Use --tags TAGS option instead.
could you please have a look again? let me know if anything is missing.
Thnaks

Copy link

@Nimesh-Msys Nimesh-Msys left a comment

Choose a reason for hiding this comment

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

And for single line statements, we could also make use of one liner conditions.

lib/chef/knife/ec2_server_create.rb Show resolved Hide resolved
@kvivek1115 kvivek1115 force-pushed the VSingh/remove-deprecated-options branch from 6921e31 to 0b53eaf Compare July 9, 2019 05:40
Vivek Singh added 5 commits July 9, 2019 11:24
 - --tags now available in chef core and has been removed from knife-ec2.
 - Removed options --tag-node-in-chef and --security-group-ids.

Signed-off-by: Vivek Singh <[email protected]>
Signed-off-by: Vivek Singh <[email protected]>
 - AMI list removes the unwanted comment and put "aws-marketplace" in option default.
 - Rename tags method with parse_aws_tags for aws_tag option.
 - Minor specs fix.

Signed-off-by: Vivek Singh <[email protected]>
@kvivek1115 kvivek1115 force-pushed the VSingh/remove-deprecated-options branch from 0b53eaf to 59b92d8 Compare July 9, 2019 05:58
@kvivek1115 kvivek1115 changed the title Remove deprecated options Remove deprecated options & set default value for ec2 ami list owner option Jul 9, 2019
Vivek Singh added 5 commits July 9, 2019 11:45
Signed-off-by: Vivek Singh <[email protected]>
Signed-off-by: Vivek Singh <[email protected]>
Signed-off-by: Vivek Singh <[email protected]>
@kvivek1115 kvivek1115 force-pushed the VSingh/remove-deprecated-options branch from a5d69dd to a5e2966 Compare July 11, 2019 12:27
@btm btm requested a review from Nimesh-Msys July 11, 2019 13:34
@tas50 tas50 merged commit 392ab66 into chef:master Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants