search

devops-coop / ansible-minecraft

Ansible role for provisioning a vanilla Minecraft server
https://galaxy.ansible.com/devops-coop/minecraft/
Apache License 2.0
57 stars 38 forks source link

Set default permission mode via stat values #19

Closed brahmlower closed 6 years ago

brahmlower commented 6 years ago

The existing value is invalid python3, and will result in a difficult to track runtime error. To support compatibility with python2.x and 3.x, I've changed this to use permission bitmasks from the stat library.

I figured I'd get approval from someone in the team before merging.

benwebber commented 6 years ago

Thanks for submitting the PR. I'll look at the Docker build.

benwebber commented 6 years ago

Thanks! Was this the only Python 3 incompatibility you found?

brahmlower commented 6 years ago

Yup! I ran pylint for python versions 2 and 3 against it to double check. It might be worth including a lint check in the CI tests. I can dedicate some time to that if you think it'd be worth while.

benwebber commented 6 years ago

Sounds great. tox makes that really easy.

The Travis configuration is "generic", so we can't use tox-travis. But we can simply install it with pip and declare our Python versions manually.

Something like this would work:

[tox]
envlist = py27,py35,py36

[testenv]
deps =
  flake8
command = flake8 library/

We could also run ansible-playbook --syntax-check and ansible-lint in the tox environment. Mind creating a new issue?