-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
[CHECK] Do not continue with other checks if integrity check fails. #413
Conversation
7d7c551
to
6dad932
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
==========================================
+ Coverage 26.98% 27.02% +0.03%
==========================================
Files 26 26
Lines 2453 2457 +4
Branches 1338 1340 +2
==========================================
+ Hits 662 664 +2
Misses 1304 1304
- Partials 487 489 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is technically an approval. Please fix the message before merging.
src/zimcheck/zimcheck.cpp
Outdated
if(enabled_tests.isEnabled(TestType::INTEGRITY)) { | ||
should_run_full_test = test_integrity(filename, error); | ||
} else { | ||
error.infoMsg("[WARNING] Integrity check is skipped. Any detected errors may in fact due to corrupted/invalid data."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... in fact be due to ...
6dad932
to
24d85cb
Compare
It make no sense to check content in a zim archive if the zim archive itself is broken and we can't read it.
24d85cb
to
875e09a
Compare
Fixed. Directly merge if you agree. |
It make no sense to check content in a zim archive if the zim archive itself is broken and we can't read it.
Fix openzim/libzim#893