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

Refactor homebrew family of modules #23780

Open
wants to merge 1 commit into
base: devel
from

Conversation

@indrajitr
Copy link
Contributor

commented Apr 20, 2017

SUMMARY

Changes to reduce duplication of common homebrew operations:

  • Extract common code to module_utils
  • homebrew, homebrew_cask depend on base class HomebrewCommon
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • homebrew
  • homebres_cask
ANSIBLE VERSION
ansible 2.4.0 (homebrew-refactor 0f092b59fe) last updated 2017/04/19 23:46:57 (GMT -500)
ADDITIONAL INFORMATION

This refactoring is along the lines of cloud modules. Once this PR is vetted and accepted, there would be more cleanups including streamlining homebrew_taps module and creating a new homebrew_bundle module all relying on HomebrewCommon.

@ansibot

This comment was marked as resolved.

Copy link
Contributor

commented Apr 20, 2017

The test ansible-test sanity --test no-basestring failed with the following error:

Command "test/sanity/code-smell/no-basestring.sh" returned exit status 1.
>>> Standard Output
./lib/ansible/module_utils/homebrew.py:        if isinstance(path, basestring):
./lib/ansible/module_utils/homebrew.py:            isinstance(brew_path, basestring)
./lib/ansible/module_utils/homebrew.py:            if isinstance(path, basestring):

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/module_utils/homebrew.py:44:1: E302 expected 2 blank lines, found 1
lib/ansible/module_utils/homebrew.py:52:1: E302 expected 2 blank lines, found 1
lib/ansible/module_utils/homebrew.py:56:1: E302 expected 2 blank lines, found 1
lib/ansible/module_utils/homebrew.py:76:23: E221 multiple spaces before operator
lib/ansible/module_utils/homebrew.py:124:13: W503 line break before binary operator

The test ansible-test sanity --test shebang failed with the following error:

Command "test/sanity/code-smell/shebang.sh" returned exit status 1.
>>> Standard Output
./lib/ansible/module_utils/homebrew.py:#!/usr/bin/python
One or more file(s) listed above have an unexpected shebang.
See test/sanity/code-smell/shebang.sh for the list of acceptable values.

click here for bot help

@ansibot

This comment was marked as resolved.

Copy link
Contributor

commented Apr 20, 2017

The test ansible-test sanity --test use-compat-six failed with the following error:

Command "test/sanity/code-smell/use-compat-six.sh" returned exit status 1.
>>> Standard Output
lib/ansible/module_utils/homebrew.py:from ansible.compat.six import string_types

click here for bot help

@indrajitr indrajitr force-pushed the indrajitr:homebrew-refactor branch Apr 20, 2017

@ansibot

This comment was marked as off-topic.

Copy link
Contributor

commented Apr 20, 2017

waiting_on: indrajitr
module: homebrew
supported_by: community
maintainers: indrajitr
changes_requested_by: null
needs_info: False
needs_revision: True
needs_rebase: False
merge_commits: []
mergeable_state: unstable
shippable_status: failure
maintainer_shipits: 0
community_shipits: 0
ansible_shipits: 0
shipit_actors: []

click here for bot help

Refactor homebrew family of modules
This reduces duplication by:
- Extracting common code to `module_utils`
- Making homebrew, homebrew_cask depend on base class `HomebrewBase`

@indrajitr indrajitr force-pushed the indrajitr:homebrew-refactor branch to cd02390 Apr 20, 2017

@s-hertel s-hertel removed the needs_triage label Apr 21, 2017

# (c) 2015-2017, Indrajit Raychaudhuri <irc+code@indrajit.com>
#
# This module is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by

This comment has been minimized.

Copy link
@resmo

resmo Apr 21, 2017

Member

Code in module_utils must not be under GNU GPL. Take BSD 2-clause please.

This comment has been minimized.

Copy link
@sivel

sivel Apr 21, 2017

Member

We've talked about this on IRC. @indrajitr will attempt to contact all of the original authors of the code to get approval for re-licensing, and if that fails, I believe the plan is to re-write all the code in a "cleanroom implementation", instead of re-licensing.

This comment has been minimized.

Copy link
@indrajitr

indrajitr Apr 24, 2017

Author Contributor

Have already received confirmation from @andrew-d (thanks Andrew!).
Waiting for the consent of @danieljaouen.

This comment has been minimized.

Copy link
@danieljaouen

danieljaouen Oct 5, 2017

Contributor

This looks good to me. Note that the homebrew_cask module has to change as per: #31372

@resmo
Copy link
Member

left a comment

needs_revision

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2017

@damselem

This comment has been minimized.

Copy link

commented Dec 10, 2017

What is the status of this PR?

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

@ansibot ansibot added the has_issue label Jul 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.