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

Remove references to host provisioning #17604

Merged
merged 1 commit into from
Jul 12, 2018

Conversation

@Fryguy
Copy link
Member

Fryguy commented Jun 19, 2018

@jrafanie This one's for you! 🎉

@jrafanie
Copy link
Member

@gmcculloug My only comment is ✂️ 🗑 🚽

Just kidding. Are you sure you got it all??? This is your opportunity to snap lots of delete lines and avoid confusion months down the line when someone finds a remnant and realizes it's not needed anymore.

@gmcculloug
Copy link
Member Author

@jrafanie My first pass was to search for miq_host_prov and MiqHostProv to find the obvious ones. If you have specific areas of concern feel free to mention them here and I'll give them increased scrutiny. I don't want you getting any credit for my deletes. 😄

@Fryguy
Copy link
Member

Fryguy commented Jun 27, 2018

cc @bdunne

@jrafanie
Copy link
Member

@gmcculloug Looks like a pretty clean deletion. I'm impressed. Do we need to have a migration to remove any rows. For example, since class MiqHostProvision < MiqRequestTask, could we have some rows in miq_request_tasks that have this MiqHostProvision class that we won't be able to instantiate after the code is gone? Looks like we use IPMI and PXE, do we have any remnants in other tables that we can't instantiate after this class is gone? miq_queue, miq_tasks, etc.

@gmcculloug
Copy link
Member Author

I believe it is very unlikely, but not impossible, that records exist. I'll look into putting a migration together. As for IPMI and PXE I would not expect any issues but will review this and the queue/task records as well.

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

✂️ 🔥 🚽 👍

@gmcculloug gmcculloug force-pushed the expunge_host_provisioning branch 2 times, most recently from d7e10a7 to 2dde3fa Compare July 4, 2018 16:24
@gmcculloug gmcculloug force-pushed the expunge_host_provisioning branch from 2dde3fa to 0164e36 Compare July 8, 2018 19:50
@miq-bot
Copy link
Member

miq-bot commented Jul 8, 2018

Checked commit gmcculloug@0164e36 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
11 files checked, 0 offenses detected
Everything looks fine. 🍰

@jrafanie
Copy link
Member

@gmcculloug Were there any other deletions you wanted to add to this PR or can we :shipit: ?

@gmcculloug
Copy link
Member Author

@jrafanie I need to finish the UI PR first, I think I over did it somewhere as my last round of changes there are causing issues when I test the UI. I'll definitely let you know when dependencies are merged and this is ready.

@bdunne
Copy link
Member

bdunne commented Jul 12, 2018

@gmcculloug All set now that the UI PR is merged?

@gmcculloug
Copy link
Member Author

Yes, I can work on the migration PR outside of this being merged. 👍

@bdunne bdunne merged commit e6e4542 into ManageIQ:master Jul 12, 2018
@bdunne bdunne added this to the Sprint 90 Ending Jul 16, 2018 milestone Jul 12, 2018
@bdunne bdunne assigned bdunne and unassigned jrafanie Jul 12, 2018
@jrafanie
Copy link
Member

✂️ 🔥 ✂️ 🔥 ✂️ 🔥

🍰 🍪 👏 🙇 😍 🎉

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.

5 participants