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

Adds vendor property to physical_server #14488

Merged
merged 2 commits into from
Apr 4, 2017
Merged

Adds vendor property to physical_server #14488

merged 2 commits into from
Apr 4, 2017

Conversation

odravison
Copy link
Contributor

Adding vendor property to physical_server by this migration

@chessbyte chessbyte requested a review from blomquisg March 24, 2017 13:40
@miq-bot
Copy link
Member

miq-bot commented Mar 24, 2017

Checked commits odravison/manageiq@ce2a6bd~...11b08b1 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍰

@odravison
Copy link
Contributor Author

odravison commented Mar 24, 2017 •
edited
Loading

@Fryguy @chessbyte
Travis got error from ManageIQ::Providers::Openstack::CloudManager::Refresher.
Do anybody know something?

@Fryguy
Copy link
Member

Fryguy commented Mar 24, 2017 •
edited
Loading

@odravison master is 🔴 , so you are inheriting that problem. I believe #14495 was put together to solve it, but it is not working. Additionally, #14505 also seems to be helping. Once those are merged, then this one should be ok.

@odravison
Copy link
Contributor Author

odravison commented Mar 24, 2017 •
edited
Loading

Odravison master is 🔴 , so you are inheriting that problem. I believe #14495 was put together to solve it, but it is not working. Additionally, #14505 also seems to be helping. Once those are merged, then this one should be ok.

Thank you, @Fryguy

@odravison
Copy link
Contributor Author

odravison commented Mar 28, 2017 •
edited
Loading

Why this PR wasn't already merged?
@blomquisg @Fryguy

@AparnaKarve
Copy link
Contributor

@Fryguy Can you please merge this?
I need this migration to be able to review/test ManageIQ/manageiq-ui-classic#755. Thanks.

@AparnaKarve AparnaKarve mentioned this pull request Mar 28, 2017
Closed
bdunne
bdunne reviewed Mar 29, 2017
View reviewed changes
db/schema.yml
@@ -6154,6 +6154,7 @@ physical_servers:
- serial_number
- field_replaceable_unit
- raw_power_state
- vendor
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between vendor and manufacturer?

Copy link
Contributor Author

@odravison odravison Mar 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, @bdunne .
vendor is a property called by all generic quadicon_helper into manageiq-ui-classic.
All entities that need render some image on interface use this vendor property
vendor is used to match files name and some times the manufacturer isn't the same.

@blomquisg blomquisg merged commit 77ebf93 into ManageIQ:master Apr 4, 2017
@blomquisg blomquisg added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 4, 2017
@isimluk isimluk mentioned this pull request Apr 5, 2017
Merged
@Fryguy
Copy link
Member

Fryguy commented Apr 7, 2017

@simaishi I am ok with this coming back to fine, but we might run into issues because of the numbering cc @carbonin I might need your help on this...it might need to be renumbered (not sure how that will affect devs)

@simaishi
Copy link
Contributor

simaishi commented Apr 7, 2017

@Fryguy thanks. @carbonin let me know about renumbering... there is also #14277 and #14644

@carbonin carbonin mentioned this pull request Apr 7, 2017
Merged
@simaishi
Copy link
Contributor

simaishi commented Apr 7, 2017

Marking as fine/conflict for now due to numbering issue/concern (#14277 (comment))

simaishi pushed a commit that referenced this pull request Apr 11, 2017
@blomquisg @simaishi
Merge pull request #14488 from Odravison/migration_to_physical_server
c9d35e1 
Adds vendor property to physical_server
(cherry picked from commit 77ebf93)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit c9d35e1f7adae6496d351dd9746abb680bf9e62a
Author: Greg Blomquist <[email protected]>
Date:   Tue Apr 4 17:01:16 2017 -0400

    Merge pull request #14488 from Odravison/migration_to_physical_server
    
    Adds vendor property to physical_server
    (cherry picked from commit 77ebf9317a62822cec8a75514df14c24f772f908)

@odravison odravison deleted the migration_to_physical_server branch February 2, 2018 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdunne bdunne bdunne left review comments

@blomquisg blomquisg Awaiting requested review from blomquisg

Assignees

@Fryguy Fryguy

Labels
fine/backported providers/physical sql migration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@odravison @miq-bot @Fryguy @AparnaKarve @simaishi @bdunne @blomquisg @chessbyte