Skip to content

No download dir for editable - #538

Merged
vphilippon merged 6 commits into
jazzband:masterfrom
dfee:no-download_dir-for-editable
Jul 6, 2017
Merged

vphilippon merged 6 commits into
jazzband:masterfrom
dfee:no-download_dir-for-editable

Conversation

@dfee

@dfee dfee commented Jun 25, 2017

Copy link
Copy Markdown
Contributor

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 #369.
@dfee

dfee commented Jun 26, 2017

Copy link
Copy Markdown
Contributor Author

@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

@vphilippon vphilippon left a comment

Copy link
Copy Markdown
Member

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.

Comment thread tests/test_cli.py Outdated


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

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

Comment thread tests/test_cli.py
'--rebuild'])
print(out.output)
assert out.exit_code == 0
assert vcs_package in out.output

Copy link
Copy Markdown
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
Copy Markdown
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

dfee commented Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

@vphilippon updated.

@vphilippon vphilippon left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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