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

WIP Replace ASSERT by real check #723

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

mgautierfr
Copy link
Collaborator

@mgautierfr mgautierfr commented Aug 11, 2022

Fixes #670

@kelson42
Copy link
Contributor

kelson42 commented Mar 8, 2023

@mgautierfr This PR is alsmot 8 months old as it was pretty time consuming to finish it. I had two days of work in my mind. Is that correct?

@mgautierfr
Copy link
Collaborator Author

At least yes.
I have put a lot of check in the low level reader using assert but we want here remove the assert (or keep it in debug, but in release is it as remove it) and add check at higher level.
This means passing through all places where data (coming from the zim file itself) may be wrong and raise a explicit exception "as soon as possible". But those places are not identified, we will probably have a lot of round trip during the review.

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

Successfully merging this pull request may close these issues.

The reader test seems to stuck in a deadlock (zim-testing-suite) for some reason.
2 participants