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.

Sign up for GitHub

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

Jump to bottom

Drop Container Component Statuses #2246

Merged
merged 1 commit into from
Sep 28, 2017
Merged

Drop Container Component Statuses #2246

merged 1 commit into from
Sep 28, 2017

Conversation

zakiva
Copy link
Contributor

@zakiva zakiva commented Sep 27, 2017 •
edited
Loading

@zakiva zakiva mentioned this pull request Sep 27, 2017
Merged
@zakiva
Copy link
Contributor Author

zakiva commented Sep 27, 2017

@simon3z @yaacov Please review

@miq-bot
Copy link
Member

miq-bot commented Sep 27, 2017

Checked commit zakiva@8b9fecb with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@zakiva
Copy link
Contributor Author

zakiva commented Sep 27, 2017

@miq-bot add_label compute/containers, cleanup

yaacov
yaacov approved these changes Sep 27, 2017
View reviewed changes
Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

LGTM

cben
cben reviewed Sep 27, 2017
View reviewed changes
app/helpers/ems_container_helper/textual_summary.rb
@@ -28,19 +28,6 @@ def textual_group_status
)
end

def textual_group_component_statuses
Copy link
Contributor

Choose a reason for hiding this comment

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

is there also list somewhere of all textual groups to call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the only list I found is here: https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ems_container_controller.rb#L119
but it does not contain component_statuses I think it was already removed in the past when there was a decision to hide it (UI only).
cc @simon3z

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was hidden in ManageIQ/manageiq#8609 and then probably didn't make it to the list above :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right!

cben
cben approved these changes Sep 27, 2017
View reviewed changes
app/helpers/ems_container_helper/textual_summary.rb
@@ -28,19 +28,6 @@ def textual_group_status
)
end

def textual_group_component_statuses
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right!

@moolitayer
Copy link

@himdel please review

@dclarizio dclarizio self-assigned this Sep 28, 2017
@dclarizio dclarizio added this to the Sprint 70 Ending Oct 2, 2017 milestone Sep 28, 2017
@dclarizio dclarizio merged commit 80b0f3b into ManageIQ:master Sep 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaacov yaacov yaacov approved these changes

@cben cben cben approved these changes

Assignees

@dclarizio dclarizio

Labels
cleanup compute/containers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zakiva @miq-bot @moolitayer @cben @yaacov @dclarizio