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

fixes #7834 - properly process incoming and updated hypervisors #4708

Merged
merged 1 commit into from Oct 17, 2014

Conversation

thomasmckay
Copy link
Member

No description provided.

System.create_hypervisor(environment.id, content_view.id, hypervisor_attrs)
created = []
consumers_attrs[:created].map do |hypervisor|
created << System.create_hypervisor(environment.id, content_view.id, hypervisor)
end if consumers_attrs[:created]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use map if you're not setting it to anything? Here you go:

created = (consumers_attrs[:created] || []).map do |hypervisor|
  System.create_hypervisor(environment.id, content_view.id, hypervisor)
end

@thomasmckay
Copy link
Member Author

@daviddavis updated, thanks! Hope you don't mind but I left the var initialization in place.

@daviddavis
Copy link
Contributor

That works. APJ

System.create_hypervisor(environment.id, content_view.id, hypervisor_attrs)
created = []
consumers_attrs[:created].each do |hypervisor|
created << System.create_hypervisor(environment.id, content_view.id, hypervisor)
end if consumers_attrs[:created]
Copy link
Contributor

Choose a reason for hiding this comment

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

My preference is for trailing if's to only be on single-line statements. This is more readable to me:

if consumers_attrs[:created]
  consumers_attrs[:created].each do |hypervisor|
    created << System.create_hypervisor(environment.id, content_view.id, hypervisor)
  end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. To be fair though, this was not introduced by @thomasmckay.

@stbenjam
Copy link
Contributor

stbenjam commented Oct 7, 2014

APJ,

I won't withold my ack because of the above, just a nitpick, but if you feel like changing it :-)

changed unneeded map to each

changed 'if' style on code

rebased

rubocop fix
thomasmckay added a commit that referenced this pull request Oct 17, 2014
fixes #7834 - properly process incoming and updated hypervisors
@thomasmckay thomasmckay merged commit 91096d1 into Katello:master Oct 17, 2014
@thomasmckay thomasmckay deleted the 7834-hypervisors branch October 17, 2014 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants