-
Notifications
You must be signed in to change notification settings - Fork 356
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
Add top row information into the storage manager quadicons #4987
Conversation
@miq-bot add_label blocker |
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. Looks good.
I'm still missing the object storage quadrants from @Loicavenel. |
@skateman there is only one option available: Cloud object store containers .. Put that on the top left |
c5bad51
to
40ef702
Compare
Thanks @Loicavenel, updated |
40ef702
to
9ebac1e
Compare
@skateman : a small spec? for the new method? |
@martinpovolny there's no new method and I'm against writing specs for declarative stuff like decorators. I can surely test if 1 = 1, but I think just turning off the coverage for the declarations makes more sense. Also the asset pipeline is turned off in testing, so some of the decorator specs would be really just 1 = 1... This special case of storage with the decision logic is wrong and I would like to get rid of it, just haven't figured out how to do so. |
@skateman : I'm sorry, but it's not that simple. You are adding code to a method that has no specs. You are adding conditions, you are calling other methods. This method is no longer a trivial declaration. I suggest that you add a very simple spec. I do not actually care what the spec tests, I don't want you to test this condition or that. I want at least this piece of code to be called from the test suite. I want a test that failed if I remove the code or if I call a non-existent method there. For example if you create a situation where Or make sure that the method is called as part of a more complex scheme, where more of decorator would be used. Pls, do not make PRs that decrease test coverage. Not even a little ;-) |
9ebac1e
to
af7e59e
Compare
@martinpovolny done |
af7e59e
to
e16922e
Compare
Checked commit skateman@e16922e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Add top row information into the storage manager quadicons (cherry picked from commit c6662af) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1650086
Hammer backport details:
|
There's no separate STI class for block & object storage managers, so I had to do this ugly hack while adding the missing information to the quadrants.
Before:
After:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1650086
@miq-bot add_label GTLs, hammer/yes, bug
@miq-bot add_reviewer @epwinchell