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 ancestry case to the genealogy parent setter #20642

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Oct 1, 2020

the multiplicity of tree like structures and their differences in behavior makes this overly complicated. we've got a few bugs with multiple parents being set and a lot of code written to address that bug in particular. we're trying to simplify all this by removing relationships.

this genealogy_parent setter needs to eventually handle ancestry cases as well.

when we destroy all relationships, we'll want to just alias this to parent=

@miq-bot assign @kbrock

relies on

#20274
for the use_ancestry? method

@d-m-u d-m-u requested review from agrare, Fryguy and kbrock as code owners October 1, 2020 23:51
@d-m-u d-m-u changed the title add ancestry case to the genealogy parent setter [wip] add ancestry case to the genealogy parent setter Oct 2, 2020
@miq-bot miq-bot added the wip label Oct 2, 2020
@d-m-u d-m-u force-pushed the improving_genealogy_parent_setter branch from 5d97e98 to 677dac3 Compare October 2, 2020 03:23
@kbrock
Copy link
Member

kbrock commented Oct 2, 2020

this code is good.

In this context, the relationships array is confusing as to what it even is.

I think this should build on the pr that establishes relationship types and has the call ancestry or other.

then the PR that is trying to get vms working will do this change.

@d-m-u d-m-u force-pushed the improving_genealogy_parent_setter branch from 677dac3 to 7750cd9 Compare October 2, 2020 20:02
@d-m-u d-m-u closed this Oct 3, 2020
@d-m-u d-m-u reopened this Oct 3, 2020
@d-m-u d-m-u closed this Nov 5, 2020
@d-m-u d-m-u reopened this Nov 5, 2020
@d-m-u d-m-u changed the title [wip] add ancestry case to the genealogy parent setter add ancestry case to the genealogy parent setter Nov 5, 2020
@miq-bot miq-bot removed the wip label Nov 5, 2020
@d-m-u
Copy link
Contributor Author

d-m-u commented Nov 5, 2020

could I please get this reviewed merged?

when we destroy all relationships, we'll want to alias
this setter to parent=
@d-m-u d-m-u force-pushed the improving_genealogy_parent_setter branch from 7750cd9 to 528509b Compare November 5, 2020 15:29
@miq-bot
Copy link
Member

miq-bot commented Nov 5, 2020

Checked commit d-m-u@528509b with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@kbrock kbrock merged commit dd2b056 into ManageIQ:master Nov 5, 2020
@d-m-u d-m-u deleted the improving_genealogy_parent_setter branch November 5, 2020 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants