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

No download dir for editable #538

Merged
merged 6 commits into from
Jul 6, 2017

Conversation

dfee
Copy link
Contributor

@dfee dfee commented Jun 25, 2017

A rebase of #385.

blueyed and others added 4 commits August 16, 2016 19:42
Using a `download_dir` will make `pip` export the cloned repo via
`reqset._prepare_file` and `git-checkout-index`, which then makes
`python setup.py egg_info` fail, when
[setuptools_scm](https://github.com/pypa/setuptools_scm) is used.

Fixes jazzband#369.
@dfee
Copy link
Contributor Author

dfee commented Jun 26, 2017

@vphilippon I'd like to get this in the next release as it replaces #385. Can we get this done?

@vphilippon vphilippon self-requested a review July 4, 2017 13:28
Copy link
Member

@vphilippon vphilippon left a comment

Choose a reason for hiding this comment

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

I've noted a few things to do with the new test.
Everything else looks good.

@@ -222,6 +222,19 @@ def test_editable_package(tmpdir):
assert 'six==1.10.0' in out.output


def test_editable_package_vcs(tmpdir):
vcs_package = 'git+git://github.com/pytest-dev/pytest-django#egg=pytest-django'
Copy link
Member

Choose a reason for hiding this comment

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

I would pin this to the current latest commit of pytest-django master branch.
The pytest-django package could stop using setuptools_scm.
Also, if they break the master branch for a while, I'd rather not have our tests fail on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

'--rebuild'])
print(out.output)
assert out.exit_code == 0
assert vcs_package in out.output
Copy link
Member

Choose a reason for hiding this comment

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

Add a check that pytest is also in the output, as it's one of pytest-django's install_requires.
We want to make sure that we still collect the subdependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@dfee
Copy link
Contributor Author

dfee commented Jul 5, 2017

@vphilippon updated.

Copy link
Member

@vphilippon vphilippon left a comment

Choose a reason for hiding this comment

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

LGTM

@vphilippon vphilippon added this to the 1.10 milestone Jul 6, 2017
@vphilippon
Copy link
Member

Thanks for the contribution!

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.

3 participants