search

freedomofpress / dangerzone

Take potentially dangerous PDFs, office documents, or images and convert them to safe PDFs
https://dangerzone.rocks/
GNU Affero General Public License v3.0
3.35k stars 152 forks source link

Minor changes #811

Closed almet closed 3 weeks ago

almet commented 1 month ago

A small pull request with changes I did while reading the current codebase.

I've added some comments directly on Github when it made sense.

almet commented 1 month ago

I converted this PR to a draft for now, until I have the tests running properly on my machine, so I can replicate what the CI is telling me :-)

almet commented 1 month ago

I've put the changes for pytest fixtures in a separate PR: #813

almet commented 1 month ago

Current linting failures are due to a bug on mypy, which will mark code as unreachable if behind a boolean assert, like this:

assert something is True
assert somethingelse # <-- this is considered unreachable

I'm considering removing the --warn-unreachable flag for the tests.

almet commented 1 month ago

Tests are failing due the latest Alpine image. This should be fixed with #812.

@apyrgio I'm curious about your feedback on disabling the mypy --warn-unreachable flag for the tests. Maybe there is a better way :-)

apyrgio commented 1 month ago

@apyrgio I'm curious about your feedback on disabling the mypy --warn-unreachable flag for the tests. Maybe there is a better way :-)

I'd be more inclined to use # type: ignore [unreachable] in affected lines, since there may be an actual case where something is unreachable in our tests, and we want t the linter to flag it.

almet commented 1 month ago

I'd be more inclined to use # type: ignore [unreachable] in affected lines, since there may be an actual case where something is unreachable in our tests, and we want t the linter to flag it.

That's indeed a better solution :-) I've updated the branch accordingly, and force-pushed.

almet commented 4 weeks ago

Just rebased this branch on top of latest main