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

Simplify requirement file parsing #7245

Merged
merged 5 commits into from
Oct 25, 2019

Conversation

chrahunt
Copy link
Member

This is the first part of a cleanup and refactoring of our requirement file parsing.

The general goal is to decouple the file/line parsing from the construction of the requirements objects. To that end, this PR reduces coupling between a few components and moves the parsing-specific logic to one place in process_line so that the extraction will be easier to review.

This PR does not go all the way to extract the parsing logic because many tests in tests.unit.test_req_file rely on the current interface. I'd rather bundle those into their own change.

This clears the way and for us to create our parser outside
the function next.
Simplifies reading the code that actually processes the line.
comes_from is only used in get_file_content, which expects to see a URL
or path, so there is no need to decorate it.
This change makes factoring out the parsing more obvious.
Decouples `process_lines` from our CLI options.
@chrahunt chrahunt added type: refactor Refactoring code skip news Does not need a NEWS file entry (eg: trivial changes) labels Oct 20, 2019
@chrahunt chrahunt marked this pull request as ready for review October 20, 2019 18:51
@chrahunt
Copy link
Member Author

Bump

@pradyunsg
Copy link
Member

(moved this out of "Build logic refactor", since IMO this isn't directly related how pip builds a distribution)

@chrahunt chrahunt merged commit be01170 into pypa:master Oct 25, 2019
@chrahunt chrahunt deleted the refactor/simplify-req-file-parsing-2 branch October 25, 2019 11:51
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Nov 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 24, 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 skip news Does not need a NEWS file entry (eg: trivial changes) type: refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants