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

Introduce new class: Pipenv #38

Merged
merged 2 commits into from
Nov 27, 2018
Merged

Conversation

nre-ableton
Copy link
Contributor

This class is designed to facilitate running a Pipenv command against
multiple versions of Python, which is something that Pipenv cannot do
by itself very gracefully. We do this by using the --python argument
to Pipenv, which assumes that the Pipfile does not contain a
requirements section which may cause Pipenv to throw an error.


This is the first step in solving https://github.com/AbletonAppDev/devtools/issues/1340, ping @AbletonDevTools/gotham-city

@nre-ableton nre-ableton force-pushed the nre/master/pipenv-multiple-python branch 2 times, most recently from df8180d to 997ab7c Compare October 12, 2018 12:19
@nre-ableton nre-ableton force-pushed the nre/master/pipenv-multiple-python branch from 997ab7c to 8a25e65 Compare October 18, 2018 07:43
@nre-ableton
Copy link
Contributor Author

re-ping @AbletonDevTools/gotham-city

Copy link
Contributor

@ala-ableton ala-ableton left a comment

Choose a reason for hiding this comment

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

I didn't look at the tests yet.

src/com/ableton/Pipenv.groovy Outdated Show resolved Hide resolved
src/com/ableton/Pipenv.groovy Outdated Show resolved Hide resolved
src/com/ableton/Pipenv.groovy Show resolved Hide resolved
vars/pipenv.groovy Outdated Show resolved Hide resolved
@nre-ableton nre-ableton force-pushed the nre/master/pipenv-multiple-python branch from 7521ff0 to 78cebe4 Compare October 26, 2018 11:44
Copy link
Contributor

@ala-ableton ala-ableton left a comment

Choose a reason for hiding this comment

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

Some final nitpicking.

src/com/ableton/Pipenv.groovy Outdated Show resolved Hide resolved
src/com/ableton/Pipenv.groovy Outdated Show resolved Hide resolved
src/com/ableton/Pipenv.groovy Outdated Show resolved Hide resolved
@nre-ableton nre-ableton force-pushed the nre/master/pipenv-multiple-python branch from 78cebe4 to 90770fe Compare October 29, 2018 11:11
@ala-ableton
Copy link
Contributor

Once there is a green PR on another repo that uses this class/singleton, LGTM 👍

@nre-ableton
Copy link
Contributor Author

@ala-ableton Thanks, but unfortunately because of pypa/pipenv#3079, that might be awhile.

This class is designed to facilitate running a Pipenv command against
multiple versions of Python, which is something that Pipenv cannot do
by itself very gracefully. We do this by using the `--python` argument
to Pipenv, which assumes that the `Pipfile` does not contain a
`requirements` section which may cause Pipenv to throw an error.
@nre-ableton nre-ableton force-pushed the nre/master/pipenv-multiple-python branch from 90770fe to 3751f42 Compare November 27, 2018 07:38
@nre-ableton
Copy link
Contributor Author

@ala-ableton Regarding the green PR on another repo: https://github.com/AbletonDevTools/ablbot/pull/186

I'm going to go ahead and merge this change, assuming there are no objections.

@nre-ableton nre-ableton merged commit caf0192 into master Nov 27, 2018
@nre-ableton nre-ableton deleted the nre/master/pipenv-multiple-python branch November 27, 2018 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants