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
Show cross linking containers links #484
Conversation
rubenvp8510
commented
Feb 24, 2017
lives_on_entity_name = _("Virtual Machine") | ||
|
||
if lives_on.kind_of?(Container) | ||
lives_on_entity_name = _("Container") |
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.
We should be having the whole label here, not just the entity name. I.e.:
label = _("Underlying Container")
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 agree, I'll do the change.
663d84a
to
2c5fd74
Compare
This PR depends on : ManageIQ/manageiq#14043 |
Checked commit rubenvp8510@2c5fd74 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
@miq-bot add_label middleware |
@miq-bot add_label ui, enhancement |
{ | ||
:label => "Underlying #{lives_on_entity_name}", |
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.
Why is "underlying" not reused as previously? now it is repeated in line 70 and in line 73
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.
Explained in a conversation above.
@rubenvp8510 @mzazrivec Is the review for this patch completed? it still says "changes requested", but it looks like all comments were addressed |
@rubenvp8510 @mzazrivec Is the review for this patch completed? it still says "changes requested", but it looks like all comments were addressed |
@abonas from my point of view this is completed and ready for merge, I've already addressed the comments of @mzazrivec. @mzazrivec Is there more comments for this PR? |
@mzazrivec @martinpovolny @bronaghs if someone can review/ack this please |
@miq-bot add_label fine/yes |
@rubenvp8510 @abonas we need a BZ to get this into |
@rubenvp8510 please see @simaishi comment. I was not sure if there's a BZ or only a JMAN? If no BZ, please create one and link it here |
@simaishi I added this BZ for backporting https://bugzilla.redhat.com/show_bug.cgi?id=1448070 |
@miq-bot add_label blocker |
Show cross linking containers links (cherry picked from commit f2837ad) https://bugzilla.redhat.com/show_bug.cgi?id=1448131
Fine backport details:
|