-
Notifications
You must be signed in to change notification settings - Fork 38
Subclass VmScan job as ManageIQ::Providers::Microsoft::InfraManager::Scanning::Job #153
Conversation
end | ||
|
||
def call_snapshot_create | ||
_log.info("Enter") |
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.
I vote for a nicer message here. :)
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.
@djberg96 while I agree, I would do that in a separate PR. I am just refactoring here - trying to change the least amount possible and ensure everything still works.
return | ||
rescue Timeout::Error | ||
msg = "Request to delete snapshot timed out" | ||
_log.error(msg) |
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.
Same issue here as I mentioned in the other one. Without a return, won't this look like it succeeded?
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.
@djberg96 I wonder how this worked prior to this refactoring.
c761049
to
25fdafd
Compare
Checked commits chessbyte/manageiq-providers-scvmm@95076ea~...25fdafd with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 app/models/manageiq/providers/microsoft/infra_manager/scanning/job.rb
|
State Machine diagram for this subclass of VmScan.