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

MiqAeNamespace should use ancestry instead of acts_as_tree #19824

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

lfu
Copy link
Member

@lfu lfu commented Feb 10, 2020

@Fryguy
Copy link
Member

Fryguy commented Feb 10, 2020

Can you run a cross repo test will all of the different branches you have?

@lfu lfu force-pushed the add_ancestry_to_miq_ae_namespace branch 2 times, most recently from fb0c7df to 2cc1456 Compare February 11, 2020 15:34
@lfu lfu force-pushed the add_ancestry_to_miq_ae_namespace branch from 2cc1456 to f6bf756 Compare March 24, 2020 14:30
@lfu lfu requested review from agrare, gtanzillo and kbrock as code owners March 24, 2020 14:30
Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

Ancestry is great when you are on a node and want to traverse all children / all parents.

But if all you want ancestry to do is to store a hierarchical key, then a basic string column will do it all and not require any other schema changes.

You now the use case better than I - so only you can say if you need to do children / parents / siblings and stuff like that.

spec/models/miq_ae_instance_spec.rb Outdated Show resolved Hide resolved
app/models/miq_ae_namespace.rb Outdated Show resolved Hide resolved
app/models/miq_ae_namespace.rb Outdated Show resolved Hide resolved
Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

Just sticking with ancestry for now is a good start.

It simplifies the next step

@lfu lfu force-pushed the add_ancestry_to_miq_ae_namespace branch from 2e23b6a to 883149b Compare April 2, 2020 16:42
@lfu lfu force-pushed the add_ancestry_to_miq_ae_namespace branch from 883149b to a6c5b9e Compare April 2, 2020 16:49
@miq-bot
Copy link
Member

miq-bot commented Apr 2, 2020

Checked commit lfu@a6c5b9e with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
6 files checked, 0 offenses detected
Everything looks fine. 👍

@bdunne bdunne closed this Apr 2, 2020
@bdunne bdunne reopened this Apr 2, 2020
@kbrock kbrock merged commit 75f60ba into ManageIQ:master Apr 2, 2020
@kbrock kbrock mentioned this pull request Apr 2, 2020
simaishi pushed a commit that referenced this pull request Apr 16, 2020
MiqAeNamespace should use ancestry instead of acts_as_tree

(cherry picked from commit 75f60ba)
@simaishi
Copy link
Contributor

Jansa backport details:

$ git log -1
commit 509b24c10ffc0530973f1739dda44da8c644bc55
Author: Keenan Brock <[email protected]>
Date:   Thu Apr 2 13:42:44 2020 -0400

    Merge pull request #19824 from lfu/add_ancestry_to_miq_ae_namespace

    MiqAeNamespace should use ancestry instead of acts_as_tree

    (cherry picked from commit 75f60baeb3c4544f77cfd48f1afb8ff16cddd906)

@lfu lfu deleted the add_ancestry_to_miq_ae_namespace branch April 17, 2020 20:18
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.

7 participants