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

Add --categories option to work with requirements txt files #5826

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

DasaniT
Copy link

@DasaniT DasaniT commented Aug 10, 2023

The issue

Fixes #5722

The fix

changed the import_requirements() and related functions to have categories. --categories now works with requirements file. whether -r <req_path> is provided or not (when only requirements.txt is available)

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@matteius
Copy link
Member

@DasaniT Could you look into possible conflicts with: #5793
We are getting very close to merging that.

@DasaniT
Copy link
Author

DasaniT commented Aug 11, 2023

@matteius I looked into the draft-no-reqlib branch and there will be conflicts (/utils/requirements.py and /tests/integration/test_install_categories.py). So, since I'm new to open source, what's the procedure here? Should I wait for that branch to merge and then rebase my branch onto master and then make the necessary changes or ...?

@matteius
Copy link
Member

matteius commented Aug 12, 2023

Should I wait for that branch to merge

@DasaniT Yeah that makes sense -- but in the mean time can you fix the CI lint (ignore the ruff job) so that the CI runner can run the tests portion?

@DasaniT
Copy link
Author

DasaniT commented Aug 13, 2023

Should I wait for that branch to merge

@DasaniT Yeah that makes sense -- but in the mean time can you fix the CI lint (ignore the ruff job) so that the CI runner can run the tests portion?

@matteius I ran pre-commit run --all-files --verbose before commiting as mentioned in https://pipenv.pypa.io/en/latest/dev/contributing/#development-setup
What am I missing here?

@kaine-bruce-dmt
Copy link

@DasaniT

image

Change the backticks to `` instead of ` for your news comment :)

@DasaniT
Copy link
Author

DasaniT commented Aug 14, 2023

@DasaniT

image

Change the backticks to `` instead of ` for your news comment :)

Thanks for your help, and sorry somehow I couldn't find the failed test:) I fixed it.

Copy link
Contributor

@oz123 oz123 left a comment

Choose a reason for hiding this comment

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

Looks good. The linked issue is a pattern that I am very familiar with.

@oz123 oz123 merged commit be8a084 into pypa:main Aug 17, 2023
18 of 19 checks passed
@matteius
Copy link
Member

matteius commented Aug 18, 2023

Darn, there were more conflicts with my branch than were reported. Edit: Sorry, couldn't think, it was exactly what was reported but confused the heck out of me, but I think I got it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specifying categories when installing from a requirements.txt
4 participants