-
Notifications
You must be signed in to change notification settings - Fork 898
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
Renamed method to delete cloud object store for consistency reasons #17143
Renamed method to delete cloud object store for consistency reasons #17143
Conversation
Checked commit romanblanco@edd5c8d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
I'm missing a link to the commit on UI here (where you use the new function name instead the old one). Other than that I think it should work out of the box, without modifying anything but this unit test on the Amazon repo: https://github.com/ManageIQ/manageiq-providers-amazon/blob/94cfa257daafab041074eb30f27794159736d3d8/spec/models/manageiq/providers/amazon/storage_manager/s3/stubbed_refresher_spec.rb#L217 |
@miha-plesko Thanks, corrected the links, all the PRs should be ready. 👍 |
Getting errors while testing:
Marking this as [WIP] for now. @miq-bot add_label wip |
@miq-bot remove_label wip |
@@ -1,7 +1,7 @@ | |||
module CloudObjectStoreObject::Operations | |||
extend ActiveSupport::Concern | |||
|
|||
def delete_cloud_object_store_object | |||
def cloud_object_store_object_delete |
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.
looks good. do we need to keep (deprecated) old link in there for a while?
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.
@romanblanco - bump
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.
@bronaghs Thanks for the reminder 👍
@kbrock I didn't find any other use of the method (see https://github.com/search?q=org%3AManageIQ+delete_cloud_object_store_object&type=Code)
So I believe it is safe just to rename it, and not keeping a link.
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.
manageiq-ui-classic/spec/controllers/application_controller/ci_processing_spec.rb
215: "delete_cloud_object_store_object",
225: :task => "delete_cloud_object_store_object",
228: controller.send(:process_objects, [object1.id, object2.id], "delete_cloud_object_store_object", "delete")
235: :task => "delete_cloud_object_store_object",
292: "delete_cloud_object_store_object",
302: :task => "delete_cloud_object_store_object",
305: controller.send(:process_objects, [object.id.to_s], "delete_cloud_object_store_object", "delete")
312: :task => "delete_cloud_object_store_object",
Those :)
EDIT: fixed by ManageIQ/manageiq-ui-classic#3607
Renamed method introduced in 81072dbb020#diff-ddea025797d4c84804a87a85c2db89a8R4 for consistency reasons.
The change allows me to generalize the
case
statement introduced in https://github.com/ManageIQ/manageiq-ui-classic/pull/498/files/524fe8c31aecc5becabedc4db880987214aeb240#r106942728Links
cc @miha-plesko