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

Refactoring code of x_get_tree_custom_kids method #13402

Merged

Conversation

hstastna
Copy link

@hstastna hstastna commented Jan 9, 2017

Refactoring code of x_get_tree_custom_kids method as the
same code needs to be in the method in tree_builder_vms_filter.rb
and in tree_builder_storage.rb;
moving the code to new methods in miq_search.rb.

This PR has to be merged with/before ManageIQ/manageiq-ui-classic#101!

@hstastna
Copy link
Author

hstastna commented Jan 9, 2017

@miq-bot add_label refactoring

@miq-bot
Copy link
Member

miq-bot commented Jan 9, 2017

Checked commit hstastna@6aa2893 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
1 file checked, 0 offenses detected
Everything looks good. 🏆

@hstastna hstastna changed the title [WIP] Refactoring code of x_get_tree_custom_kids method Refactoring code of x_get_tree_custom_kids method Jan 10, 2017
@hstastna hstastna changed the title Refactoring code of x_get_tree_custom_kids method [WIP] Refactoring code of x_get_tree_custom_kids method Jan 10, 2017
@hstastna hstastna changed the title [WIP] Refactoring code of x_get_tree_custom_kids method Refactoring code of x_get_tree_custom_kids method Jan 10, 2017
@chessbyte chessbyte added core and removed wip labels Jan 10, 2017
when "my" # My filters
visible_to_current_user
else
raise "Error: #{type} is not a proper type!"
Copy link
Member

Choose a reason for hiding this comment

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

I am thinking about this line. Do we need an error message?

If we do then it should probably say "filter type" not just "type".

Copy link
Author

@hstastna hstastna Jan 11, 2017

Choose a reason for hiding this comment

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

Thank you for your comment. I am not sure if we need an error message there, I considered it to be a good idea.. to think about all the cases that could occur somehow in the future..
I think, one shouldn't rely on the fact that maybe we don't need it. We should be sure if we need it or not. Maybe you or anybody else knows for sure..
If we need it, I can improve the error message.

depends on ManageIQ/manageiq-ui-classic#98

Refactoring code of x_get_tree_custom_kids method as the
same code needs to be in the method in tree_builder_vms_filter.rb
and in tree_builder_storage.rb;
moving the code to new methods in miq_search.rb.
@hstastna hstastna force-pushed the x_get_tree_custom_kids_REFACTORING1 branch from 6aa2893 to 08942e9 Compare January 16, 2017 14:58
@martinpovolny martinpovolny added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 26, 2017
@chessbyte chessbyte removed this from the Sprint 53 Ending Jan 30, 2017 milestone Jan 27, 2017
@martinpovolny martinpovolny merged commit c2d53aa into ManageIQ:master Feb 13, 2017
@martinpovolny martinpovolny modified the milestones: Roadmap, Sprint 54 Ending Feb 13, 2017 Feb 13, 2017
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.

5 participants