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

Use pipenv for managing developement virtualenv #410

Merged
merged 1 commit into from
Mar 13, 2018
Merged

Conversation

ob-stripe
Copy link
Contributor

@ob-stripe ob-stripe commented Mar 13, 2018

r? @brandur-stripe
cc @stripe/api-libraries

This PR ended up being larger than I intended, so here's some helpful information:

  • pipenv is a fancy new tool for managing development virtualenvs. It's fairly recent, but is officially recommended by python.org. (It's also developed by Kenneth Reitz of requests fame, and is used by requests itself.)
  • Pipfile and Pipfile.lock work similarly to Gemfile in Ruby projects. I've committed the lock file because it's recommended (cf. git - Should Pipfile.lock be committed to version control? pypa/pipenv#598) and is what's done in requests.
  • Added a Makefile for common operations
  • Updated .travis.yml to use the new make targets
  • Updated tox.ini to set up a separate environment for flake8 linting

@stripe-ci
Copy link

@ob-stripe ob-stripe changed the base branch from master to integration-v2 March 13, 2018 14:30
@ob-stripe ob-stripe force-pushed the ob-pipenv branch 4 times, most recently from b031435 to c03dec8 Compare March 13, 2018 15:04
Pipfile Outdated

[packages]

"e1839a8" = {path = ".", editable = true}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line basically means that pipenv will run python setup.py install, which will pull the install dependencies from setup.py according to the Python environment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea what "e1839a8" is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a short hash of the path (in this case .) automatically generated by pipenv to avoid collisions if you have multiple entries. I think it can actually be anything unique -- I'll try renaming it to stripe for clarity.

Makefile Outdated
@@ -0,0 +1,15 @@
init:
pip install --upgrade pipenv
pipenv install --dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind if we remove the init target in favor of just having this duplicated in .travis.yml and the README.md? I feel like many Python people will already have have a pipenv available, and there are other recommended ways of installing it (e.g., I used Homebrew as recommended on their page).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@brandur-stripe
Copy link
Contributor

Very cool! I just tried it to make sure it wasn't too much trouble, and your instructions in the README.md went off without a hitch :)

I left a few comments above, but otherwise LGTM.

@ob-stripe
Copy link
Contributor Author

Okay, I made the requested changed. Going to pull this in.

@ob-stripe ob-stripe merged commit f18389d into integration-v2 Mar 13, 2018
@ob-stripe ob-stripe mentioned this pull request Mar 13, 2018
7 tasks
@ob-stripe ob-stripe deleted the ob-pipenv branch March 27, 2018 09:28
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.

None yet

3 participants