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

stop the addition of the branch name in the #egg=... fragment #3312

Merged
merged 4 commits into from Jan 18, 2016

Conversation

Projects
None yet
3 participants
@sbidoul
Contributor

sbidoul commented Dec 18, 2015

fixes #3296
fixes #3292

Review on Reviewable

def get_src_requirement(self, dist, location, find_tags=False):
repo = self.get_url(location)
if repo is None:
return None
parts = repo.split('/')
# FIXME: why not project name?
egg_project_name = dist.egg_name().split('-', 1)[0]

This comment has been minimized.

@xavfernandez

xavfernandez Dec 19, 2015

Contributor

This seems quite odd, I'd say we should be using the canonicalized project_name...

@xavfernandez

xavfernandez Dec 19, 2015

Contributor

This seems quite odd, I'd say we should be using the canonicalized project_name...

This comment has been minimized.

@sbidoul

sbidoul Dec 19, 2015

Contributor

Where can we obtain that canonicalized project_name? There is dist.key which is dist.project_name.lower(). egg_name() replaces dashes by underscores, so I guess that behaviour must be preserved.

@sbidoul

sbidoul Dec 19, 2015

Contributor

Where can we obtain that canonicalized project_name? There is dist.key which is dist.project_name.lower(). egg_name() replaces dashes by underscores, so I guess that behaviour must be preserved.

This comment has been minimized.

@xavfernandez

xavfernandez Jan 7, 2016

Contributor

There is pip.utils.canonicalize_name which is equivalent to dist.key. It is used for the name matching in https://github.com/pypa/pip/blob/69d9044/pip/req/req_install.py#L451 .

But to be consistent with what pip used to produce, we should indeed keep replace('-', '_').
So I guess egg_name() is fine, it's just that I find it less clear.

@xavfernandez

xavfernandez Jan 7, 2016

Contributor

There is pip.utils.canonicalize_name which is equivalent to dist.key. It is used for the name matching in https://github.com/pypa/pip/blob/69d9044/pip/req/req_install.py#L451 .

But to be consistent with what pip used to produce, we should indeed keep replace('-', '_').
So I guess egg_name() is fine, it's just that I find it less clear.

@sbidoul

This comment has been minimized.

Show comment
Hide comment
@sbidoul

sbidoul Dec 19, 2015

Contributor

Test failures are unrelated.

Contributor

sbidoul commented Dec 19, 2015

Test failures are unrelated.

@xavfernandez xavfernandez added this to the 8.0 milestone Jan 7, 2016

@xavfernandez

This comment has been minimized.

Show comment
Hide comment
@xavfernandez

xavfernandez Jan 7, 2016

Contributor

Could you rebase it on develop ?

Contributor

xavfernandez commented Jan 7, 2016

Could you rebase it on develop ?

Show outdated Hide outdated pip/vcs/subversion.py Outdated
@xavfernandez

This comment has been minimized.

Show comment
Hide comment
@xavfernandez

xavfernandez Jan 8, 2016

Contributor

#3347 is complementary to fix #3292

Contributor

xavfernandez commented Jan 8, 2016

#3347 is complementary to fix #3292

@xavfernandez

This comment has been minimized.

Show comment
Hide comment
@xavfernandez

xavfernandez Jan 8, 2016

Contributor

@pypa/pip-developers I'd like to merge this, particularly to enable #3153 (which will be partially backed off in pip8).
No objection ? :)

Contributor

xavfernandez commented Jan 8, 2016

@pypa/pip-developers I'd like to merge this, particularly to enable #3153 (which will be partially backed off in pip8).
No objection ? :)

@sbidoul

This comment has been minimized.

Show comment
Hide comment
@sbidoul

sbidoul Jan 14, 2016

Contributor

rebased

Contributor

sbidoul commented Jan 14, 2016

rebased

dstufft added a commit that referenced this pull request Jan 18, 2016

Merge pull request #3312 from sbidoul/no-branch-in-egg-name-sbi
stop the addition of the branch name in the #egg=... fragment

@dstufft dstufft merged commit 3ae13b2 into pypa:develop Jan 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sbidoul sbidoul deleted the sbidoul:no-branch-in-egg-name-sbi branch Jan 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment