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

Displayed Physical Server Hardware info #1272

Merged

Conversation

MaysaMacedo
Copy link
Member

@MaysaMacedo MaysaMacedo commented May 4, 2017

This PR is able to:
-Display Physical Server hardware information (total memory and total cores)

image

@miq-bot
Copy link
Member

miq-bot commented May 5, 2017

This pull request is not mergeable. Please rebase and repush.

@MaysaMacedo MaysaMacedo force-pushed the show_physical_server_hardware_info branch from 2dc7266 to 1e866b0 Compare May 8, 2017 19:29
@AparnaKarve
Copy link
Contributor

@MaysaMacedo Please add a screenshot in the first PR comment.
LGTM otherwise.

@dclarizio @martinpovolny Tested in UI and can be merged.
(spec failures seem unrelated)

@dclarizio
Copy link

@MaysaMacedo @AparnaKarve this travis failure DOES look related:

  2) PhysicalServerController#show should eq 200
     Failure/Error: it { expect(response.status).to eq(200) }
       expected: 200
            got: 500
       (compared using ==)
     # ./spec/controllers/physical_server_controller_spec.rb:15:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:75:in `block (3 levels) in <top (required)>'
     # ./spec/manageiq/spec/support/evm_spec_helper.rb:28:in `clear_caches'
     # ./spec/spec_helper.rb:75:in `block (2 levels) in <top (required)>'

@AparnaKarve
Copy link
Contributor

@dclarizio You are right. I just saw the first error which affected a lot of PRs yesterday.

@MaysaMacedo Let me know if you need help resolving the spec failure.

@AparnaKarve
Copy link
Contributor

@MaysaMacedo To resolve the spec error, please add the following in the #show spec in physical_server_controller_spec --

computer_system = FactoryGirl.create(:computer_system, :hardware => FactoryGirl.create(:hardware))
physical_server = FactoryGirl.create(:physical_server, :computer_system => computer_system)

@AparnaKarve
Copy link
Contributor

AparnaKarve commented May 9, 2017

@MaysaMacedo While merging the 'master' commit sometimes addresses the merge conflict error (referring to f447558), the recommended approach to resolve the merge conflict would be to rebase your current branch with master.
It just keeps the commit history cleaner.
Could you please rebase and repush? Thanks.

@MaysaMacedo MaysaMacedo force-pushed the show_physical_server_hardware_info branch 3 times, most recently from 58c4d34 to bb29fe4 Compare May 9, 2017 20:34
@MaysaMacedo
Copy link
Member Author

@AparnaKarve When rebasing with master 47 commits are created and bigger conflicts are generated

@AparnaKarve
Copy link
Contributor

@MaysaMacedo Let's discuss how to rebase in gitter.

@MaysaMacedo MaysaMacedo force-pushed the show_physical_server_hardware_info branch from bb29fe4 to 2ac2836 Compare May 9, 2017 20:46
@miq-bot
Copy link
Member

miq-bot commented May 9, 2017

Checked commits MaysaMacedo/manageiq-ui-classic@f052252~...2ac2836 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@AparnaKarve
Copy link
Contributor

@MaysaMacedo LGTM

@dclarizio OK to merge

@dclarizio dclarizio merged commit 08fba62 into ManageIQ:master May 9, 2017
@dclarizio dclarizio added this to the Sprint 61 Ending May 22, 2017 milestone May 9, 2017
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.

4 participants