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

Improve dependencies compatibility definition, and testing #75

Closed
thibaudcolas opened this issue Aug 30, 2017 · 3 comments
Closed

Improve dependencies compatibility definition, and testing #75

thibaudcolas opened this issue Aug 30, 2017 · 3 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@thibaudcolas
Copy link
Collaborator

Raised by @loicteixeira in https://github.com/springload/draftjs_exporter/pull/74/files#r135967185. Opening as a separate issue because it's worth discussing and there is no point in holding that PR for this.

At the moment, draftjs_exporter defines its dependencies as:

dependencies = [
'beautifulsoup4>=4.4.1,<5',
'html5lib>=0.999,<=1.0b10',
]
lxml_dependencies = [
'lxml>=3.6.0',
]

Those ranges are purposefully big (we want to support as many versions as possible for something as fundamental to people's tech stacks), which is good, but as @loicteixeira puts it then it would make sense to test accordingly, with the lower and upper bounds at least.


More info to help in the decision,

    "beautifulsoup4>=4.5.1",
    "html5lib>=0.999,<1",

Finally, bear in mind that our usage of the APIs of those dependencies is very small (HTML string -> DOM nodes conversion, DOM nodes -> HTML string conversion, create nodes, append child to node), which means that the potential breakage would only likely be in how those engines handle specific content, which is hard to test for. We do however have a small test suite of "potential engine quirks": https://github.com/springload/draftjs_exporter/blob/master/tests/engines/test_engines_differences.py

@thibaudcolas thibaudcolas added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Aug 30, 2017
@thibaudcolas thibaudcolas added this to the Nice to have milestone Aug 30, 2017
@loicteixeira
Copy link
Contributor

loicteixeira commented Aug 31, 2017

I reckon it's safer to have an upper bound for both for now as we can always increase it later once we confirm everything is working rather than having to rush a patch.

beautifulsoup4>=4.4.1,<5 and lxml>=3.6.0,<4 would be a good start.

If you're agree with that I'll update the test-matrix and documentation when I have a moment.

Edit: I realize now that the name of the package beautifulsoup4 kinda implies that there will never be a version 5 and that they would instead release a separate beautifulsoup5 package but we can't rely solely on assumptions.

@loicteixeira loicteixeira self-assigned this Aug 31, 2017
@thibaudcolas
Copy link
Collaborator Author

Sounds good!

@thibaudcolas
Copy link
Collaborator Author

lxml 4.0.0 has been released in september and I can confirm 4.1.1 works fine, so I'd suggest going for lxml>=3.6.0,<5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants