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

Added Embedded Ansible Content plugin #17096

Merged

Conversation

syncrou
Copy link
Contributor

@syncrou syncrou commented Mar 6, 2018

Adds a new Vmdb plugin for AnsibleContent.

The registered plugin contains the following paths: @path and @roles_path

Vmdb::Plugins.instance.instance_variable_get(:@registered_ansible_content)
 => [#<Vmdb::Plugins::AnsibleContent:0x007fb45d9c6170 @roles_path=#<Pathname:/Users/dbomhof/syncrou/manageiq-content/content/ansible/roles>, @path=#<Pathname:/Users/dbomhof/syncrou/manageiq-content/content/ansible>>]

Dependent on: ManageIQ/manageiq-content#254

Thanks to @gmcculloug for helping with the refactor of the Automate and EmbeddedAnsible registration methods

https://bugzilla.redhat.com/show_bug.cgi?id=1539762

@syncrou
Copy link
Contributor Author

syncrou commented Mar 6, 2018

@miq-bot add_label enhancement

@miq-bot assign @gmcculloug

cc- @gtanzillo @Fryguy

@registered_automate_domains << AutomateDomain.new(domain_directory)
end
end

def register_embedded_ansible_content(engine)
registered_content_directories(engine, "ansible") do |domain_directory|
Copy link
Member

Choose a reason for hiding this comment

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

The domain_directory is a carry-over from the automate method. Suggest content_directory for this one.

@gmcculloug gmcculloug assigned Fryguy and unassigned gmcculloug Mar 6, 2018
@syncrou syncrou force-pushed the add_embedded_ansible_content_plugin branch from 6324dd3 to 360a4f4 Compare March 6, 2018 16:40
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.

👍 on the refactoring.

Should this be embedded_ansible_content or just ansible_content? I'm leaning toward ansible_content since the "embedded" part is just an implementation detail. What do you think? cc @Fryguy

@gtanzillo
Copy link
Member

I prefer ansible_content fwiw

@syncrou syncrou force-pushed the add_embedded_ansible_content_plugin branch from 360a4f4 to 4c1d80c Compare March 7, 2018 18:42
@syncrou
Copy link
Contributor Author

syncrou commented Mar 7, 2018

I changed everything to AnsibleContent per the request.

@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2018

Checked commit syncrou@4c1d80c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@bdunne bdunne merged commit 35f2f4e into ManageIQ:master Mar 7, 2018
@bdunne bdunne added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 7, 2018
@bdunne bdunne assigned bdunne and unassigned Fryguy Mar 7, 2018
@JPrause
Copy link
Member

JPrause commented Apr 10, 2018

@miq-bot add_label blocker

simaishi pushed a commit that referenced this pull request Apr 12, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 01375d9eecae8c3783d12e486ccb9944b90c9da5
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Wed Mar 7 16:21:21 2018 -0500

    Merge pull request #17096 from syncrou/add_embedded_ansible_content_plugin
    
    Added Embedded Ansible Content plugin
    (cherry picked from commit 35f2f4e82a3a64fd8c307dee2ea9bd2929c0bcda)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1566658

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.

8 participants