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 all instances of towhat #18222

Closed
juliancheal opened this issue Nov 20, 2018 · 10 comments
Closed

Remove all instances of towhat #18222

juliancheal opened this issue Nov 20, 2018 · 10 comments
Assignees

Comments

@juliancheal
Copy link
Member

juliancheal commented Nov 20, 2018

We've deprecated towHat and replacing it with resource_type. There are many instances of this across not only ManageIQ repo, but also the Schema repo, and the UI Classic (and others?).

These are the order we'll need to merge the PRs in order to reduce Travis failures and therefore sad developers.

TL:DR

Merge schema, then merge the core and ui-classic prs. Use deprecate_attribute :towhat, :resource_type in models to allow for any issues during transition.

Completed PRs

Closed PRs

@NickLaMuro
Copy link
Member

NickLaMuro commented Dec 19, 2018

@juliancheal something I found out when I was doing ManageIQ/manageiq-api#516 is that only MiqSchedule had the towhat column removed, but there are other models that use this pattern, as the manageiq-schema PR suggests

I would have to update my PR, or create a followup that works with ManageIQ/manageiq-schema#314 to handle the extra column updates. Possibly update the description to reflect that for the API PR?

Also, I have #18241 which seems like it would probably conflict with #18165 that you have put together. Should I close mine? Of note, I think your's requires the changes in ManageIQ/manageiq-schema#314 where mine is only handling MiqSchedule's use of towhat.

@juliancheal
Copy link
Member Author

@NickLaMuro hey Nick, our plan was to merge the schema changes, then as soon as travis is green merge the manageiq core, then ui repos straight after.

I'm guessing we're going to have to do this in the new year now, as I think both LJ and Brandon are out.

Yeah I think if it's ok with you to close #18241, as it's covered in #18165. 🎅🏽

@NickLaMuro
Copy link
Member

@juliancheal Cool, sounds good.

@miq-bot
Copy link
Member

miq-bot commented Jun 24, 2019

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@chessbyte
Copy link
Member

@juliancheal @NickLaMuro what is the status of this issue?

@NickLaMuro
Copy link
Member

@chessbyte Unsure. I was just commenting previously regarding a deprecation warning PR that I opened and was in potential conflict, but I didn't have an idea on the general plan for this effort.

@juliancheal
Copy link
Member Author

@chessbyte #18165 needs rebasing, then I think that should be good to go. Next we need to change the schema, I closed the two PRs I had on @Fryguy's advice. I need to double check with him, but I think the plan was to simply rename the column.

@juliancheal
Copy link
Member Author

@chessbyte All PRs rebased, schema pr re-opened. Just waiting for merges whenever @Fryguy is ready I guess :) Happy Friday.

jrafanie added a commit to jrafanie/manageiq-api that referenced this issue Mar 6, 2020
@miq-bot
Copy link
Member

miq-bot commented Jun 11, 2020

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.

@miq-bot miq-bot added the stale label Jun 11, 2020
@Fryguy Fryguy removed the stale label Jul 6, 2020
@jrafanie
Copy link
Member

jrafanie commented Jul 6, 2020

Note, the core PR here lays out the changes needed in core, api (ui-classic, schema. It was decided in the core PR that while it appears to work, the changes were not insignificant for a minor change. We'll need to re-evaluate if this is desired in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants