Refactor amazon_customizerequest method for VM Provisioning. #628
Conversation
Pull Request Test Coverage Report for Build 4104
💛 - Coveralls |
Pull Request Test Coverage Report for Build 4151
💛 - Coveralls |
pkomanek
force-pushed
the
amazon_customize_request_refactoring
branch
2 times, most recently
from
5a9d5e6 to
093bd03
Compare
| end | ||
| end | ||
| end | ||
| def main(mapping: false) |
There was a problem hiding this comment.
I think it is better keep the main method signature same as other ruby methods.
def main
...
mapping = false
process_customization if mapping
...
end
def process_customization
set_options
set_customization_template
end
| end | ||
|
|
||
| def log(level, msg, update_message = false) | ||
| @handle.log(level, msg.to_s) |
There was a problem hiding this comment.
Is to_s necessary? How about @handle.log(level, msg)?
|
|
||
| def log(level, msg, update_message = false) | ||
| @handle.log(level, msg.to_s) | ||
| @handle.root['miq_provision'].message = msg.to_s if @handle.root['miq_provision'] && update_message |
pkomanek
force-pushed
the
amazon_customize_request_refactoring
branch
2 times, most recently
from
f4a4cab to
6afe308
Compare
pkomanek
changed the title
| end | ||
|
|
||
| def product | ||
| @product ||= template.operating_system['product_name'].downcase rescue nil |
There was a problem hiding this comment.
@pkomanek This refactoring, while it did not introduce the rescue statements, should be able to easily remove them.
For most the initial object is guaranteed to be valid because the accessor method is raising an error. Here the template method will always return a template object or raise an error. So we just need to worry about operating_system and the remaining items being nil.
If we use the . notation instead of ['product_name'] to reference the property you can use safe navigation to achieve this:
@product ||= template.operating_system&.product_name&.downcase| return | ||
| end | ||
|
|
||
| customization_template_search_by_function = "#{prov.type}_#{prov.get_tags[:function]}" rescue nil |
There was a problem hiding this comment.
The prov method will raise an error if it cannot return the object, so we can just remove the rescue nil at the end.
|
|
||
| customization_template_search_by_function = "#{prov.type}_#{prov.get_tags[:function]}" rescue nil | ||
| customization_template_search_by_template_name = template.name | ||
| customization_template_search_by_ws_values = ws_values[:customization_template] rescue nil |
There was a problem hiding this comment.
The ws_values method defaults to an empty hash so we can just remove the rescue nil at the end.
| :name => customization_template.name, | ||
| :desc => customization_template.description | ||
| }) | ||
| prov.set_customization_template(customization_template) rescue nil |
There was a problem hiding this comment.
This is the only one I'm not really sure about and may want to keep in this refactoring and address later.
pkomanek
force-pushed
the
amazon_customize_request_refactoring
branch
from
6afe308 to
cfb36fd
Compare
pkomanek
force-pushed
the
amazon_customize_request_refactoring
branch
from
cfb36fd to
c4e93c0
Compare
|
Some comments on commits pkomanek/manageiq-content@2843cd3~...c4e93c0 spec/content/automate/ManageIQ/Cloud/VM/Provisioning/StateMachines/Methods.class/methods/amazon_customizerequest_spec.rb
|
|
Checked commits pkomanek/manageiq-content@2843cd3~...c4e93c0 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 content/automate/ManageIQ/Cloud/VM/Provisioning/StateMachines/Methods.class/methods/amazon_customizerequest.rb
|
Refactoring the method and creating tests.
This PR is based on the issue below.
ManageIQ/manageiq#12038
@miq-bot add_label refactoring
@miq-bot assign @tinaafitz