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

Add server profile template table for PhysicalServer #673

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

eilam20
Copy link
Contributor

@eilam20 eilam20 commented Nov 18, 2022

No description provided.

@agrare
Copy link
Member

agrare commented Nov 21, 2022

Merge branch 'ManageIQ:master' into add_server_profile_template

@eilam20 it looks like you did a merge here instead of a rebase can you drop that commit and rebase your branch on master rather than merging master into your branch?

See https://github.com/ManageIQ/guides/blob/master/developer_setup/git_workflow.md#git-configuration for our recommended git configuration

Comment on lines 1 to 39
require_migration

# This is mostly necessary for data migrations, so feel free to delete this
# file if you do no need it.
describe CreatePhysicalServerProfileTemplates do
# let(:my_model_stub) { migration_stub(:MyModel) }

migration_context :up do
# Create data
#
# Example:
#
# my_model_stub.create!(attributes)

migrate

# Ensure data exists
#
# Example:
#
# expect(record).to have_attributes()
end

migration_context :down do
# Create data
#
# Example:
#
# my_model_stub.create!(attributes)

migrate

# Ensure data exists
#
# Example:
#
# expect(MyModel.count).to eq(0)
end
end
Copy link
Member

Choose a reason for hiding this comment

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

If you're simply adding a table you don't need to have a spec test (there is nothing to test aside from syntax which is already tested when we migrate up and down)

Choose a reason for hiding this comment

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

Done 👍

@agrare
Copy link
Member

agrare commented Nov 22, 2022

Now you have two merge commits @tzurvaza 😆 I can rebase if you want but you should get used to rebasing on master

@agrare
Copy link
Member

agrare commented Nov 22, 2022

Looks like you don't have allow maintainer edits enabled so you'll have to fix this yourself

@eilam20 eilam20 force-pushed the add_server_profile_template branch 2 times, most recently from 2f3112c to bb8888c Compare November 22, 2022 19:33
@agrare
Copy link
Member

agrare commented Nov 22, 2022

Minor but this is -schema, the last fix commit should probably be rebased into the original commit since it isn't really helpful to the history or showing the progression of work

@eilam20
Copy link
Contributor Author

eilam20 commented Nov 22, 2022

@agrare I dropped the commits and pushed it again with the rebase.

@eilam20
Copy link
Contributor Author

eilam20 commented Nov 22, 2022

Minor but this is -schema, the last fix commit should probably be rebased into the original commit since it isn't really helpful to the history or showing the progression of work

ok, done

@miq-bot
Copy link
Member

miq-bot commented Nov 22, 2022

Checked commit Autosde@f850dad with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

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

4 participants