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

Consider making typer an optional requirement (for CLI only) #99

Open
barseghyanartur opened this issue Dec 22, 2022 · 3 comments
Open
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request

Comments

@barseghyanartur
Copy link

Because:

  • CLI is not the core functionality
  • typer depends on click and it's quite often hard to find a suitable version of click which isn't conflicting with other deps.
@justindujardin justindujardin added enhancement New feature or request dependencies Pull requests that update a dependency file labels Jan 11, 2024
@justindujardin
Copy link
Owner

Yes, this is a good idea.

I don't have time to work on it right now, but I can outline an approach for anyone who wants to submit a PR.

@barseghyanartur
Copy link
Author

Well, since I'm using Pathy and I'd love to get rid of yet another annoying dependency which only causes problems and multiplies dependency hell (click).

If you agree, I can submit a PR (trivial). Or if have any specific requirements/steps-to-follow, please, outline them.

@justindujardin
Copy link
Owner

justindujardin commented Jan 11, 2024

The only requirements I have relate to automation and developer experience.

Automation

The test suite needs to pass (file-system adapter is good enough to open a PR).

You can setup a virtual env and run the tests with:

sh tools/setup.sh # creates .env/ 
sh tools/test.sh # pytest

The linting task has to pass; usually, running format.sh first is enough:

sh tools/format.sh
sh tools/lint.sh

The suite includes tests for the CLI, so they need to be run twice when the code is optional. This kind of testing is already done for the client adapters, so you can copy that approach. See test_package.sh for how it's put together. You just need to make sure that the CLI file is run once when click isn't installed and then again when it is so that the code cov hits all the paths for the error message with installation instructions. This might not require anything from you, I don't recall 😅

Developer Experience

Since the CLI part will be optional, we need to provide the user with instructions to install the proper dependencies if they use the CLI without them installed. You can reference any of the client adapters to see the pattern for this. It goes like:

  • User actually tries to use the thing that's not installed
  • Catch the error, and display a message containing the pip install instructions to fix the problem

And don't forget to add the "cli" extras in setup.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants