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

Adds tenant and tentanttag support #61

Merged
merged 1 commit into from Sep 7, 2016
Merged

Conversation

spuder
Copy link
Contributor

@spuder spuder commented Sep 6, 2016

Description

Allows cookbook to add a machine to a tenant or add tenant tags

I've manually tested this. Not sure how to automate testing this.

Contribution Check List

  • All tests pass.
  • New functionality includes testing.
  • New functionality has been documented in the README.

@@ -150,6 +150,8 @@
environment = new_resource.environment
config_path = new_resource.config_path
service_name = service_name(instance)
tenant = new_resource.tenant
Copy link
Member

Choose a reason for hiding this comment

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

plural since this is an array

@@ -37,3 +37,5 @@
attribute :api_key, kind_of: String
attribute :roles, kind_of: Array
attribute :environment, kind_of: String, default: node.chef_environment
attribute :tenants, kind_of: Array, default: nil
Copy link
Member

@brentm5 brentm5 Sep 7, 2016

Choose a reason for hiding this comment

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

why not default to empty array? If we default them to empty array the other PR actualy isn't needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried empty array initially. Gives a deprecation warning

       Deprecated features used!
         An attempt was made to change tenant_tags from [] to nil by calling tenant_tags(nil). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil. at 1 location:
           - C:/Users/ADMINI~1/AppData/Local/Temp/kitchen/cache/cookbooks/nd-octopus/recipes/tentacle.rb:15:in `block in from_file'

@brentm5
Copy link
Member

brentm5 commented Sep 7, 2016

LGTM just going to wait for #60 before merging

@brentm5
Copy link
Member

brentm5 commented Sep 7, 2016

Just do a down merge and rebase and this should be good to go

@spuder spuder force-pushed the feature/tenant branch 2 times, most recently from 359cbb9 to 3656caf Compare September 7, 2016 22:01
Default to empty array
changes to ruby variable names

Updates readme

changes default to nil
@spuder
Copy link
Contributor Author

spuder commented Sep 7, 2016

Rebased. Ready 🚀

@brentm5 brentm5 merged commit 656839a into cvent:master Sep 7, 2016
@spuder spuder deleted the feature/tenant branch September 7, 2016 23:22
@spuder spuder mentioned this pull request Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants