Skip to content

Separate pip search command from operation #2410

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Separate pip search command from operation #2410

wants to merge 2 commits into from

Conversation

msabramo
Copy link
Contributor

@msabramo msabramo commented Feb 9, 2015 •
edited by BrownTruck
Loading

By extracting the logic into pip.operations.search, the hope is that
folks could do a search programmatically more easily.

It also has greater separation of concerns and should allow people to
work in parallel with less chance of merge conflicts.

Continuing work started in #2173 and #2404.

@msabramo msabramo mentioned this pull request Feb 9, 2015
Closed
@msabramo
Copy link
Contributor Author

msabramo commented Feb 9, 2015

Hmmm. I don't understand the Travis CI failure. Those PEP8 failures don't look related to my code and they don't happen for me locally.

[marca@marca-mac2 pip]$ git log -n 1
commit 7a17792e07047a577a09ec60113cbf5e1671b59b
Author: Marc Abramowitz <[email protected]>
Date:   Mon Feb 9 07:38:37 2015 -0800

    Separate pip search command from operation

    By extracting the logic into `pip.operations.search`, the hope is that
    folks could do a `search` programmatically more easily.

    It also has greater separation of concerns and should allow people to
    work in parallel with less chance of merge conflicts.

    Continuing work started in #2173 and #2404.
[marca@marca-mac2 pip]$ tox -e pep8,py3pep8
GLOB sdist-make: /Users/marca/dev/git-repos/pip/setup.py
pep8 inst-nodeps: /Users/marca/dev/git-repos/pip/.tox/dist/pip-6.1.0.dev0.zip
pep8 runtests: PYTHONHASHSEED='1303968444'
pep8 runtests: commands[0] | flake8 .
py3pep8 inst-nodeps: /Users/marca/dev/git-repos/pip/.tox/dist/pip-6.1.0.dev0.zip
py3pep8 runtests: PYTHONHASHSEED='1303968444'
py3pep8 runtests: commands[0] | flake8 .
___________________________________________________________________________________ summary ____________________________________________________________________________________
  pep8: commands succeeded
  py3pep8: commands succeeded
  congratulations :)

@msabramo msabramo force-pushed the extract_search_operation branch from 7a17792 to 0d33ec5 Compare February 9, 2015 16:19
@msabramo
Copy link
Contributor Author

msabramo commented Feb 9, 2015

I believe #2412 will fix the PEP 8 test failures.

@msabramo
tox.ini: Pin pep8<1.6
7538303 
pep8==1.6 added some additional checks that pip currently fails.

This prevents Travis CI failures until folks can evaluate whether the
issues should be fixed in the code or ignored.
@msabramo msabramo force-pushed the extract_search_operation branch from 0d33ec5 to cc73f9b Compare February 10, 2015 06:07
msabramo added a commit to msabramo/pip that referenced this pull request Feb 10, 2015
@msabramo
Separate pip wheel command from operation
6614c4d 
By extracting the logic into `pip.operations.wheel`, the hope is that
folks could do a `wheel` programmatically more easily.

It also has greater separation of concerns and should allow people to
work in parallel with less chance of merge conflicts.

Continuing work started in pypa#2173, pypa#2404, and pypa#2410.
@msabramo
Copy link
Contributor Author

msabramo commented Mar 5, 2015

Thoughts?

@xavfernandez
Copy link
Member

Seems fine, following the other operations commits but the pep8 part doesn't belong here.

@msabramo msabramo force-pushed the extract_search_operation branch from cc73f9b to 24b931f Compare March 5, 2015 23:03
@msabramo
Copy link
Contributor Author

msabramo commented Mar 5, 2015

Ah yes, the pep8 stuff was because my PR was failing because pep8 got upgraded and added new checks that other code besides mine was failing. @dstufft has since fixed this with e324b27, 0bc8aa5 c1c638b, etc., so I now can remove the pep8 stuff from this PR, which I just did. Thanks!

xavfernandez
xavfernandez reviewed Mar 5, 2015
View reviewed changes
pip/operations/search.py

def search(query, options, build_session_func):
index_url = options.index
with build_session_func(options) as session:
Copy link
Member

Choose a reason for hiding this comment

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

The session building part could stay on the commands side. This would also allow to only have the index_url argument instead of the whole options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I just updated to do as you said.

@msabramo
Separate pip search command from operation
41175a1 
By extracting the logic into `pip.operations.search`, the hope is that
folks could do a `search` programmatically more easily.

It also has greater separation of concerns and should allow people to
work in parallel with less chance of merge conflicts.

Continuing work started in pypa#2173 and pypa#2404.
@msabramo msabramo force-pushed the extract_search_operation branch from 24b931f to 41175a1 Compare April 14, 2015 16:22
@msabramo
Copy link
Contributor Author

Rebased and updated according to @xavfernandez's comments. Ready for review.

@dstufft dstufft closed this May 18, 2016
@dstufft
Copy link
Member

dstufft commented May 18, 2016

Accidentally closed this, reopening. Sorry!

@dstufft dstufft reopened this May 18, 2016
@BrownTruck
Copy link
Contributor

Hello!

As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the master branch. However, this Pull Request was made against the develop branch so it will need to be resubmitted against master. Unfortunately, this pull request does not cleanly merge against the current master branch.

If you do nothing, this Pull Request will be automatically closed by @BrownTruck since it cannot be merged.

If this pull request is still valid, please rebase it against master (or merge master into it) and resubmit it against the master branch, closing and referencing the original Pull Request.

If you choose to rebase/merge and resubmit this Pull Request, here is an example message that you can copy and paste:

By extracting the logic into pip.operations.search, the hope is that
folks could do a search programmatically more easily.

It also has greater separation of concerns and should allow people to
work in parallel with less chance of merge conflicts.

Continuing work started in #2173 and #2404.

---

*This was migrated from pypa/pip#2410 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*

@BrownTruck
Copy link
Contributor

This Pull Request was closed because it cannot be automatically reparented to the master branch and it appears to have bit rotted.

Please feel free to re-open it or re-submit it if it is still valid and you have rebased it onto master or merged master into it.

@BrownTruck BrownTruck added auto-bitrotten PRs that died because they weren't updated and removed asked to reparent labels May 26, 2016
@BrownTruck BrownTruck closed this May 26, 2016
@msabramo
Copy link
Contributor Author

msabramo commented Jun 2, 2016

@dstufft: Would you find this useful? I don't want to waste time reparenting it, if it's not something that would have value.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
auto-bitrotten PRs that died because they weren't updated auto-locked Outdated issues that have been locked by automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@msabramo @xavfernandez @dstufft @BrownTruck