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

vultr: new module utils and common docs #30868

Merged
merged 2 commits into from
Nov 1, 2017
Merged

vultr: new module utils and common docs #30868

merged 2 commits into from
Nov 1, 2017

Conversation

resmo
Copy link
Contributor

@resmo resmo commented Sep 25, 2017

SUMMARY

shared code and docs for vultr modules. Modules will be in follow up PRs.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

vultr

ANSIBLE VERSION
2.5
ADDITIONAL INFORMATION

closes #30796

/cc @bcoca

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 cloud community_review In order to be merged, this PR must follow the community review workflow. feature_pull_request module This issue/PR relates to a module. module_utils/ needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. support:community This issue/PR relates to code supported by the Ansible community. labels Sep 25, 2017
@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Sep 28, 2017
@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 6, 2017
# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
# USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a short form for the BSD license now:

# (c) 2017, René Moser <mail@renemoser.net> 
# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)

Copy link
Contributor

@abadger abadger Oct 31, 2017

Choose a reason for hiding this comment

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

New module_utils should have the standard boilerplate as well:

from __future__ import absolute_import, division, print_function
__metaclass__ = type

Once you use the __metaclass__ = type, you no longer need to have class Foo(object):. You can just use class Foo: instead.

)
if 'VULTR_API_CONFIG' in os.environ:
paths += (os.path.expanduser(os.environ['VULTR_API_CONFIG']),)
if not any([os.path.exists(c) for c in paths]):
Copy link
Contributor

Choose a reason for hiding this comment

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

note, this is not a blocker but you can use a generator expression here instead of a list comprehension. it could save on some system calls (calling os.path.exists) but I doubt that will be a noticable optimization so it's really just style.

@abadger
Copy link
Contributor

abadger commented Oct 31, 2017

Everything I saw boils down to style. The note about adding the boilerplate is a blocker and changing the license header to the short version is nice for consistency and reduces the over-the-wire size but everything else is trivial and doesn't need to change if you don't want to.

+1 to merge when you've added the boilerplate and made whichever other of the changes you agree with.

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Oct 31, 2017
@resmo
Copy link
Contributor Author

resmo commented Oct 31, 2017

@abadger thx for review

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed shipit This PR is ready to be merged by Core labels Nov 1, 2017
@resmo resmo merged commit c098c42 into ansible:devel Nov 1, 2017
@resmo resmo deleted the feature/vultr_utils branch November 1, 2017 19:17
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 5, 2018
@dagwieers dagwieers added the vultr Vultr community label Feb 11, 2019
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module_utils/ module This issue/PR relates to a module. new_module This PR includes a new module. support:community This issue/PR relates to code supported by the Ansible community. vultr Vultr community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants