-
Notifications
You must be signed in to change notification settings - Fork 897
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
Add copying tag as optional choice #19206
Conversation
@miq-bot assign @tinaafitz |
fd73a5d
to
7ddb0d7
Compare
@miq-bot add_label enhancement, ivanchuk/yes, services, changelog/yes |
7ddb0d7
to
8fe3fa7
Compare
|
||
new_template = service_template.template_copy | ||
|
||
expect(new_template.tags.count).to eq(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of eq(0)
use be_zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know that!
|
||
new_template = service_template.template_copy(:copy_tags => true) | ||
|
||
expect(new_template.tags.count).to eq(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be checking that the tags on the new_template
match the original template, not just the count.
You can do that with expect(new_template.tags).to match_array(service_template.tags)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this!
When service template is copied, tags might also need to be copied as requested. This change adds an optional filed to allow tags to be copied as well.
8fe3fa7
to
09159b3
Compare
Checked commit astrozzc@09159b3 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tinaafitz Please review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@astrozzc Looks good. :-)
🎉 Thanks for the PR @astrozzc |
When service template is copied, tags might also need to be copied as well. This change adds an optional filed to allow tags to be copied as well.
https://bugzilla.redhat.com/show_bug.cgi?id=1740399