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

Miscellaneous code clean-ups #57

Merged
merged 4 commits into from
May 27, 2020
Merged

Miscellaneous code clean-ups #57

merged 4 commits into from
May 27, 2020

Conversation

ylep
Copy link
Member

@ylep ylep commented Apr 30, 2020

Some of the new code requires six >= 1.12, but an older version is installed with some Python versions on Travis (Python 2.7 and 3.7).

We could add six >= 1.12 to the Travis configuration, but shouldn't soma-workflow be free of external dependencies, so that it works out of the box when installed on a server with the auto-install feature? On the other hand, the auto-install feature seems to be broken anyway (#56).

I see two ways forward:

  1. Really require six >= 1.12, add it to the Travis config, and find a way to make the auto-install functional or deprecate it entirely;

  2. Get rid of the dependency of six by incorporating six.py in the code. This is allowed by the licence, and actually we did it already for casa-distro.

@populse/soma what is your preference betwee solution 1 / solution 2?

These changes will require six >= 1.12.0 because they make use of
six.ensure_binary, six.ensure_str, and six.ensure_text, which were
introduced in that version.
@ylep ylep requested a review from a team April 30, 2020 14:22
@denisri
Copy link
Contributor

denisri commented Apr 30, 2020

We already require six >= 1.13 in other projects, and agreed on that, so I guess it chould be the same here.

@ylep ylep marked this pull request as ready for review May 27, 2020 14:00
@ylep ylep merged commit 9232050 into master May 27, 2020
@ylep ylep deleted the remove_python_version_tests branch May 27, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants