Skip to content

Install optional requirements if available #18

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

Merged
merged 2 commits into from
Jul 11, 2022

Conversation

tekktrik
Copy link
Member

@tekktrik tekktrik commented Jul 11, 2022

The plan for switching to pyproject.toml is use optional_requirements.txt to hold optional dependencies for libraries. This may be an issue for things like building documentation and pylint, so this additional step checks for the file and pip installs it so.

While I don't think it would cause any issues if rolled out, this is totally fine as a draft PR until pyproject.toml gets rolled out, but wanted to get eyes on it early!

@tekktrik tekktrik requested review from kattni and makermelissa July 11, 2022 16:34
Copy link
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

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

Took me a minute to follow this change. This makes a lot of sense. Good catch on realising this step would need to be done!

I'll leave it up to you as to when to merge this. If it's not interfering with anything else, I would say we can merge it now.

Also, it may be worth merging this now, manually updating one library with optional requirements, merging that PR, and making sure it all works together.

Copy link
Collaborator

@makermelissa makermelissa 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 to me.

@tekktrik
Copy link
Member Author

Sounds good, I'll merge this now then and target a library with optional dependencies. Thanks y'all!

@tekktrik tekktrik marked this pull request as ready for review July 11, 2022 21:12
@tekktrik tekktrik merged commit af180c1 into adafruit:master Jul 11, 2022
@tekktrik tekktrik deleted the dev/optional-requirements branch July 12, 2023 00:12
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.

3 participants