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

remove Pipenv in favor of pip-tools, security updates #372

Merged
merged 7 commits into from
May 30, 2019
Merged

Conversation

redshiftzero
Copy link
Contributor

@redshiftzero redshiftzero commented May 14, 2019

Closes #370
Closes #308
Closes #371

Testing

  • Verify hashes are the same from the removed Pipfile.lock compared to requirements.txt except the two dependencies updated here
  • Verify dev setup works for you following README updates
  • Verify static analysis, CVE safety check, test jobs pass in CI

Required diff review

  • urllib3 1.24 -> 1.24.3
  • sqlalchemy 1.2.13 -> 1.3.3

Check prior to merge

Makefile Outdated Show resolved Hide resolved
@kushaldas
Copy link
Contributor

This will break the package building steps. As our package index (wheel builds) + debian packaging works based on Pipfile.lock and also based on the requirements.txt with private wheel hashes.
We should discuss this more.

@redshiftzero
Copy link
Contributor Author

Yep I know, see this message in the PR description (there's another branch in the debian packaging repo called bye-pipenv which has my wip thus far):

In draft mode because there need to be accompanying PRs over in the securedrop-debian-packaging repo and securedrop-proxy prior to us merging this here

@kushaldas
Copy link
Contributor

Yep I know, see this message in the PR description (there's another branch in the debian packaging repo called bye-pipenv which has my wip thus far):

Ah, I assume it means that pipenv's new feature about updating only one package does not work as it should?

@kushaldas
Copy link
Contributor

@kushaldas
Copy link
Contributor

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

This is good. Approved 🦄 🦄 🦄 🦄 🌈

@redshiftzero redshiftzero marked this pull request as ready for review May 28, 2019 17:15
@redshiftzero
Copy link
Contributor Author

(force pushed to resolve conflicts only)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants