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

Update xhtml2pdf #2047

Closed
wants to merge 1 commit into from
Closed

Update xhtml2pdf #2047

wants to merge 1 commit into from

Conversation

timobrembeck
Copy link
Member

@timobrembeck timobrembeck commented Feb 1, 2023

Short description

Finally, xhtml2pdf released a new version including @charludo's upstream contribution 🎉

I did not add a new test case for the specific fixed issue, maybe this would be a good idea to make sure it isn't re-introduced in future versions.

Proposed changes

  • Update xhtml2pdf
  • Update test files

Side effects

Resolved issues

Fixes: #1498
Fixes: #2022

Also, it removes the vulnerable dependency future (see https://github.com/digitalfabrik/integreat-cms/security/dependabot/20)


Pull Request Review Guidelines

@timobrembeck timobrembeck requested a review from a team as a code owner February 1, 2023 14:17
@codeclimate
Copy link

codeclimate bot commented Feb 1, 2023

Code Climate has analyzed commit 988cd16 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 75.3% (0.0% change).

View more on Code Climate.

@seluianova
Copy link
Contributor

@timoludwig

If I understood correctly, this update also resolves #2022 / xhtml2pdf/xhtml2pdf#662, right? Could you check whether that's true?

Well, kind of.
But #2022 was a minor cosmetic issue that didn't affect readability much.

I believe xhtml2pdf/xhtml2pdf#538 was a bigger issue, and now it is broken again.
The lines are reversed, it affects the meaning.

@seluianova
Copy link
Contributor

@timobrembeck Hey Timo, do you think it might make sense to keep the changes in test_pdf_export.py and revert the others maybe, in order to close this PR?
I guess we wouldn't want to upgrade xhtml2pdf to the current version anyway.

@timobrembeck
Copy link
Member Author

@seluianova The changes in the tests only make sense when the xhtml2pdf version is upgraded (since the newer version depends on pypdf instead of PyPDF3).
But yes, sadly it seems as I don't have time to contribute to xhtml2pdf in the near future 😢

I'm going to close this PR but keep the branch, so in case we upgrade eventually, I can reuse the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⛔ blocked Blocked by external dependency
Projects
None yet
2 participants