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

lxml is required #9

Closed
charliewolf opened this issue Apr 27, 2016 · 6 comments
Closed

lxml is required #9

charliewolf opened this issue Apr 27, 2016 · 6 comments

Comments

@charliewolf
Copy link

In order to parse html emails, lxml is required. This should be added to setup.py.

@thomasst
Copy link
Member

It's an optional dependency. If you're just interested in parsing plain text emails, you don't need to install it.

@charliewolf
Copy link
Author

It should at least be documented. Getting an ImportError when deploying is not the greatest way to find out such things.

@thomasst
Copy link
Member

Where? The README already says:

quotequail has no mandatory dependencies, however using HTML methods require libxml.

@charliewolf
Copy link
Author

Aha, I am stupid and didn't see that.

I still think adding it to setup.py is the right approach, but it's not a particularly strongly held opinion.

@thomasst
Copy link
Member

I just don't want to make it a mandatory dependency. Also, your tests should have caught it before deploying it.

@thomasst
Copy link
Member

Feel free to reopen if you have specific suggestions on how to make it clearer, keeping in mind that libxml should stay as an optional dependency

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

No branches or pull requests

2 participants