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

Added subdirectory editable option, small refactoring. #1082

Merged
merged 7 commits into from Oct 6, 2013

Conversation

niedbalski
Copy link
Contributor

  • Small refactoring on pip/req.py.
  • Added _strip_postfix, _build_req_from_url, _build_editable_options functions.
  • Added test for install a package from a subdirectory.

@@ -37,7 +37,8 @@
class InstallRequirement(object):

def __init__(self, req, comes_from, source_dir=None, editable=False,
url=None, as_egg=False, update=True, prereleases=None):
url=None, as_egg=False, update=True, prereleases=None,
editable_options={}):
Copy link

Choose a reason for hiding this comment

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

Passing mutable objects as default values can lead to bugs, better to pass None and create the empty dict in the method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@niedbalski
Copy link
Contributor Author

Hey @pnasrat , merge?

@freyes
Copy link

freyes commented Aug 8, 2013

This is a pretty useful feature for my current use case, we have a single repository for several python packages, the layout looks like this:

foo_pkg
  setup.py
bar_pkg
  setup.py

Being foo and bar python packages., it would be great to install them using pip.

@pnasrat
Copy link
Contributor

pnasrat commented Aug 12, 2013

Sorry been caught up with life stuff - can you make sure this is mergable then I'll take another look.

@niedbalski
Copy link
Contributor Author

Hey @pnasrat:

Synced changes from upstream. Ready to merge.

@davehunt
Copy link

+1 for this awesome feature!

"""
Strip req postfix ( -dev, 0.2, etc )
"""
## FIXME: use package_to_requirement?
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity what's stopping you use that? Not a blocker from me but just curious

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see it's an extracted method keeping the original comment.

@pnasrat
Copy link
Contributor

pnasrat commented Aug 14, 2013

+1 from me @qwcode @hltbra @pfmoore happy for this to get merged?

@pnasrat
Copy link
Contributor

pnasrat commented Aug 20, 2013

I think this is pretty much ready but can you:

Add a description of the change to CHANGES.txt and also add documentation for the feature in docs/usage.rst

@niedbalski
Copy link
Contributor Author

@pnasrat done.

@niedbalski
Copy link
Contributor Author

Hey @pnasrat, any news on this merge?

@pnasrat
Copy link
Contributor

pnasrat commented Sep 10, 2013

Sorry I missed the last few updates in my inbox for this bug somehow. It looks like it's not mergable atm. I know this has taken longer than I like if you have time to get it mergable now do it and I'll merge otherwise I can try pull and fix up myself on the weekend.

@davehunt
Copy link

Any update on this?

@pnasrat
Copy link
Contributor

pnasrat commented Sep 27, 2013

Looking at merging here

https://github.com/pnasrat/pip/tree/subdirectory-editable

Currently running tests.

@pnasrat pnasrat merged commit d16281f into pypa:develop Oct 6, 2013
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants