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

create a physical rack table #156

Merged
merged 1 commit into from
Feb 16, 2018
Merged

Conversation

felipedf
Copy link
Member

This create a new table physical_racks and provides a :physical_rack_id column to physical_servers table.

@felipedf
Copy link
Member Author

@Fryguy

@agrare
Copy link
Member

agrare commented Feb 15, 2018

@miq-bot assign @Fryguy

@@ -0,0 +1,5 @@
class AddPhysicalRackToPhysicalServers < ActiveRecord::Migration[5.0]
def change
add_column :physical_servers, :physical_rack_id, :bigint
Copy link
Member

Choose a reason for hiding this comment

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

Don't think you need/want to do this in its own migration, adding a column to the new table in the above migration is fine /cc @Fryguy opinions?

@felipedf
Copy link
Member Author

@agrare resolved

@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2018

Checked commit felipedf@2be9ef9 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@Fryguy Fryguy merged commit 5f8bd66 into ManageIQ:master Feb 16, 2018
@Fryguy Fryguy added the schema label Feb 16, 2018
@Fryguy Fryguy added this to the Sprint 80 Ending Feb 26, 2018 milestone Feb 16, 2018
@Fryguy
Copy link
Member

Fryguy commented Feb 16, 2018

@felipedf I just realized this didn't include an ems_ref column, which is generally required if the table is to be filled in during inventory refresh.

@felipedf felipedf deleted the create_rack branch May 4, 2018 18:37
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