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 homebrew_bundle module #46401

Closed
wants to merge 5 commits into from

Conversation

danieljaouen
Copy link
Contributor

SUMMARY

Adds the homebrew_bundle module.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

homebrew_bundle

ANSIBLE VERSION
± ansible --version
ansible 2.8.0.dev0 (homebrew_bundle 90b48de229) last updated 2018/10/02 11:28:46 (GMT -400)
  config file = /Users/dan/.ansible.cfg
  configured module search path = ['/Users/dan/src/ansible/library']
  ansible python module location = /Users/dan/src/ansible/lib/ansible
  executable location = /Users/dan/src/ansible/bin/ansible
  python version = 3.6.3 (default, Oct 21 2017, 11:37:52) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]
ADDITIONAL INFORMATION

N/A

@ansibot

This comment has been minimized.

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 ci_verified Changes made in this PR are causing tests to fail. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. labels Oct 2, 2018
@danieljaouen
Copy link
Contributor Author

This is failing with E312: no RETURN provided. What exactly do I need to provide here?

@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Oct 2, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 2, 2018
@danieljaouen
Copy link
Contributor Author

Still curious about the RETURN string, as I’d like to populate it with valid info instead of a blank comment.

Sent with GitHawk

@jborean93
Copy link
Contributor

@danieljaouen it doesn't look like you are returning anything outside of the defaults like changed so having an empty RETURN string is fine. If you are wanting to return specific values then that needs to be done in the module code and documented as such. Some background info on return values

If you have further questions around this, I highly recommend you post a question on the mailing list or on IRC;

@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Oct 4, 2018
@danieljaouen
Copy link
Contributor Author

@jborean93 Thanks for the info! I just wanted to make sure I wasn't skimming on the documentation for the module.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 12, 2018
@maxamillion
Copy link
Contributor

@mattclay ping - how would one go about writing macOS integration tests to exercise this functionality? Thanks!

@mattclay
Copy link
Member

mattclay commented Nov 2, 2018

@maxamillion Creating an integration test for macOS isn't much different than any other integration test. The test should be named after the module being tested, as usual.

The test can use available facts to determine if tests need to be skipped due to the platform running the tests. As an optimization it should have entries to skip some platforms in the aliases file for the test:

skip/rhel
skip/freebsd

It may also help to take a look at some of the logic used for macOS when dealing with brew:

- name: Install prerequisites for backports.lzma when using python2 (OSX)
block:
- name: Find brew binary
command: which brew
register: brew_which
- name: Get owner of brew binary
stat: path="{{ brew_which.stdout }}"
register: brew_stat
- name: "Install package"
homebrew:
name: xz
state: present
update_homebrew: no
become: yes
become_user: "{{ brew_stat.stat.pw_name }}"
when:
- ansible_python_version.split('.')[0] == '2'
- ansible_os_family == 'Darwin'

@maxamillion
Copy link
Contributor

@danieljaouen if you don't mind, can you add some integration tests to exercise the functionality added here? Thank you!

@danieljaouen
Copy link
Contributor Author

@maxamillion I've written out a basic integration test for this, but I need to test it on my local machine. How can I run just that single integration test?

@mattclay
Copy link
Member

mattclay commented Nov 8, 2018

@danieljaouen From the checkout of your fork:

source hacking/env-setup
ansible-test integration homebrew_bundle -v

This assumes the test is under test/integration/targets/homebrew_bundle/.

options:
state:
description:
- the state of the homebrew bundle
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- the state of the homebrew bundle
- The intended state of the homebrew bundle.

state:
description:
- the state of the homebrew bundle
choices: ['installed', 'dumped', 'cleanup']
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
choices: ['installed', 'dumped', 'cleanup']
type: str
choices: [ cleanup, dumped, installed ]

default: installed
file_path:
description:
- The path of the Brewfile. Defaults to ~/.Brewfile.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- The path of the Brewfile. Defaults to ~/.Brewfile.
- The path of the Brewfile.

file_path:
description:
- The path of the Brewfile. Defaults to ~/.Brewfile.
required: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
required: false
type: path

path:
description:
- "A ':' separated list of paths to search for 'brew' executable.
Since a package (I(formula) in homebrew parlance) location is prefixed relative to the actual path of I(brew) command,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Since a package (I(formula) in homebrew parlance) location is prefixed relative to the actual path of I(brew) command,
- Since a package (I(formula) in homebrew parlance) location is prefixed relative to the actual path of I(brew) command,

- "A ':' separated list of paths to search for 'brew' executable.
Since a package (I(formula) in homebrew parlance) location is prefixed relative to the actual path of I(brew) command,
providing an alternative I(brew) path enables managing different set of packages in an alternative location in the system."
default: '/usr/local/bin'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
default: '/usr/local/bin'
type: path
default: /usr/local/bin

default: '/usr/local/bin'
install_options:
description:
- options flags to install a package
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- options flags to install a package
- Options flags to install a package.
type: list

install_options:
description:
- options flags to install a package
aliases: ['options']
Copy link
Contributor

@dagwieers dagwieers Jan 23, 2019

Choose a reason for hiding this comment

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

Suggested change
aliases: ['options']
aliases: [ options ]

Why add an alias from the start ?

description:
- options flags to install a package
aliases: ['options']
version_added: "2.8"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
version_added: "2.8"

version_added: "2.8"
'''
EXAMPLES = '''
# Install from a Brewfile.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Install from a Brewfile.
- name: Install from a Brewfile.

'''
EXAMPLES = '''
# Install from a Brewfile.
- homebrew_bundle:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- homebrew_bundle:
homebrew_bundle:

state: installed
file_path: ~/.Brewfile

# Dump to a Brewfile.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Dump to a Brewfile.
- name: Dump to a Brewfile.

Copy link
Contributor

@dagwieers dagwieers left a comment

Choose a reason for hiding this comment

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

A few cosmetic changes.

file_path: ~/.Brewfile

# Dump to a Brewfile.
- homebrew_bundle:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- homebrew_bundle:
homebrew_bundle:

file_path: ~/.Brewfile

# Cleanup from a Brewfile.
- homebrew_bundle:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- homebrew_bundle:
homebrew_bundle:

state: dumped
file_path: ~/.Brewfile

# Cleanup from a Brewfile.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Cleanup from a Brewfile.
- name: Cleanup from a Brewfile.


# This file is part of Ansible
#
# Ansible is free software: you can redistribute it and/or modify
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a one-line statement.

# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)

@@ -0,0 +1,21 @@
# (c) 2018, Daniel Jaouen <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# (c) 2018, Daniel Jaouen <[email protected]>
# Copyright: (c) 2018, Daniel Jaouen <[email protected]>

@ansibot
Copy link
Contributor

ansibot commented Feb 1, 2019

@ansibot ansibot added the packaging Packaging category label Feb 17, 2019
@ansibot
Copy link
Contributor

ansibot commented Feb 25, 2019

@ansibot
Copy link
Contributor

ansibot commented Jun 1, 2019

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 3, 2020
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Mar 27, 2020
@ansibot ansibot added collection Related to Ansible Collections work collection:community.general support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 29, 2020
@Akasurde
Copy link
Member

Akasurde commented Jul 2, 2020

@danieljaouen Thank you very much for your interest in Ansible. This plugin/module is no longer maintained in this repository and has been migrated to community.general. Could you please migrate this PR to the above-mentioned repo?

If you have further questions please stop by IRC or the mailing list:

Thanks,

needs_info

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Jul 2, 2020
@ansibot
Copy link
Contributor

ansibot commented Aug 3, 2020

@danieljaouen This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

@Akasurde
Copy link
Member

Hi @danieljaouen, Thank you very much for your interest in Ansible. This plugin/module is no longer maintained in this repository and has been migrated to https://github.com/ansible-collections/community.general

If you have further questions please stop by IRC or the mailing list:

* IRC: #ansible on irc.freenode.net
* mailing list: https://groups.google.com/forum/#!forum/ansible-project

needs_info

@Akasurde Akasurde closed this Aug 20, 2020
@ansible ansible locked as resolved and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 collection:community.general collection Related to Ansible Collections work macos macOS community module This issue/PR relates to a module. needs_info This issue requires further information. Please answer any outstanding questions. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_module This PR includes a new module. new_plugin This PR includes a new plugin. packaging Packaging category stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants