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

Dial back Pythonfinder integration effort #2582

Merged
merged 57 commits into from
Jul 30, 2018
Merged

Dial back Pythonfinder integration effort #2582

merged 57 commits into from
Jul 30, 2018

Conversation

uranusjr
Copy link
Member

Let’s try this a little at a time…

This only uses Pythonfinder in find_a_system_python, and leaves everything else alone. I hope this approach will make rewrites easier to tackle.

@uranusjr uranusjr force-pushed the pythonfinder-lite branch 2 times, most recently from 4d54c61 to 893fb8f Compare July 14, 2018 11:13
@techalchemy techalchemy force-pushed the pythonfinder-lite branch 9 times, most recently from 9c68041 to ff985cc Compare July 16, 2018 06:31
pipenv/core.py Outdated
'{} -c "import sys; print(sys.executable)"'.format(line),
)
if not isinstance(output, str):
output = output.decode(sys.getdefaultencoding())
Copy link
Member

Choose a reason for hiding this comment

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

fs_str(output)

But we don't need to handle this anyway

pipenv/core.py Outdated
c = delegator.run(
cmd, block=False, timeout=PIPENV_TIMEOUT, env=pip_config,
)
c.block()
Copy link
Member

Choose a reason for hiding this comment

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

If we don't block here and we pass block=False we can't read the output...

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it should be indented inside the with block?

Copy link
Member

Choose a reason for hiding this comment

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

makes sense to me, I just put it there because of no reason in particular

@techalchemy
Copy link
Member

just rebased this, I think I'm probably good with it if you are (I can cut 1.0 of pythonfinder if we are satisfied)

techalchemy and others added 22 commits July 25, 2018 17:04
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
This is still not working though.
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
techalchemy and others added 2 commits July 25, 2018 17:23
Signed-off-by: Dan Ryan <dan@danryan.co>
@uranusjr uranusjr added the PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. label Jul 30, 2018
@uranusjr uranusjr merged commit a853814 into master Jul 30, 2018
@uranusjr uranusjr deleted the pythonfinder-lite branch July 30, 2018 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants