-
Notifications
You must be signed in to change notification settings - Fork 357
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
Remove the second (type) argument from TreeBuilder classes #5495
Conversation
4af5fae
to
5150e85
Compare
5150e85
to
093709d
Compare
Checked commit skateman@093709d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
It's cool someone (you) is moving forward with the tree. |
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.
The code looks good. But I have not tested this in the UI.
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.
Tested, LGTM 👍
@miq-bot add_label changelog/yes |
Fixed 2 issues - Fixed `wrong number of arguments (given 4, expected 3)` error, removed 'type' argument that was being passed in to tree builder code, was introduced by ManageIQ#5495 - Fixed `comparison of Integer with String failed` error for `@assign[:new][:objects].sort!`, changed to push integer into objects array to fix comparison error on the sort when selecting nodes in the tree in 'Selections' box. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1707871 Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1707886
Fixed 2 issues - Fixed `wrong number of arguments (given 4, expected 3)` error, removed 'type' argument that was being passed in to tree builder code, was introduced by ManageIQ#5495 - Fixed `comparison of Integer with String failed` error for `@assign[:new][:objects].sort!`, changed to push integer into objects array to fix comparison error on the sort when selecting nodes in the tree in 'Selections' box. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1707871 Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1707886
Now the
type
andname
arguments are completely redundant and thetype
is not used anywhere so it is time to remove it.There are some tree initialization calls where the two arguments were swapped in order. This might cause some problems with
x_tree
andx_active_tree
in the area as the name of the tree might not had the_tree
suffix. Here are those:TreeBuilderSections
TreeBuilderProtect
TreeBuilderClusters
TreeBuilderDatastores
TreeBuilderSmartproxyAffinity
Also I might have missed some cases where metaprogramming comes into the game so would be nice to go through some of the non-explorer trees a little.
@miq-bot add_label trees, cleanup, hammer/no
@miq-bot add_reviewer @ZitaNemeckova