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

Supporting differents identify leds name of physical servers on refresh #115

Merged
merged 1 commit into from
Nov 21, 2017

Conversation

douglasgabriel
Copy link
Member

@douglasgabriel douglasgabriel commented Nov 20, 2017

This PR is able to:

  • Support identify leds with name "identification" on refresh

https://bugzilla.redhat.com/show_bug.cgi?id=1515453

@douglasgabriel
Copy link
Member Author

@AndreyMenezes

Copy link
Member

@AndreyMenezes AndreyMenezes left a comment

Choose a reason for hiding this comment

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

LGTM

@AndreyMenezes
Copy link
Member

@miq-bot add_label gaprindashvili/yes

@AndreyMenezes
Copy link
Member

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Nov 20, 2017
end
loc_led_state
identification_led_name = %w(Identify Identification)
identification_led = leds&.find { |led| identification_led_name.include?(led["name"]) }
Copy link
Member

Choose a reason for hiding this comment

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

leds.to_a.find would be better, we still backport to versions with rubies without &.

identification_led_name = %w(Identify Identification)
identification_led = leds&.find { |led| identification_led_name.include?(led["name"]) }
return identification_led["state"] unless identification_led.nil?
""
Copy link
Member

Choose a reason for hiding this comment

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

Does this have to be "" or is nil okay?
How about just identification_led.try(:[], "state")

@miq-bot
Copy link
Member

miq-bot commented Nov 21, 2017

Checked commit douglasgabriel@64fd5c8 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@douglasgabriel
Copy link
Member Author

Thanks @agrare for the feedback, I've already adapt the code

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍

@agrare agrare merged commit c0b7889 into ManageIQ:master Nov 21, 2017
@agrare agrare added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 21, 2017
simaishi pushed a commit that referenced this pull request Nov 21, 2017
Supporting differents identify leds name of physical servers on refresh
(cherry picked from commit c0b7889)

https://bugzilla.redhat.com/show_bug.cgi?id=1515898
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 8672bf13f3e4a4b246deb256cb681a90a7f074df
Author: Adam Grare <[email protected]>
Date:   Tue Nov 21 08:11:57 2017 -0500

    Merge pull request #115 from douglasgabriel/support_indentify_leds
    
    Supporting differents identify leds name of physical servers on refresh
    (cherry picked from commit c0b7889796d0e37d5b93b1db751092cb8c23d565)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1515898

@douglasgabriel douglasgabriel deleted the support_indentify_leds branch November 22, 2017 11:04
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.

None yet

6 participants