openzim / warc2zim

Command line tool to convert a file in the WARC format to a file in the ZIM format
https://pypi.org/project/warc2zim/
GNU General Public License v3.0
44 stars 4 forks source link

Fix detection of encoding again #314

Closed benoit74 closed 3 months ago

benoit74 commented 3 months ago

Fix #312

Changes

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.49%. Comparing base (4c12681) to head (9933304).

:exclamation: Current head 9933304 differs from pull request most recent head b1c8a35

Please upload reports for the commit b1c8a35 to get more accurate results.

Files Patch % Lines
src/warc2zim/utils.py 91.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #314 +/- ## ========================================== + Coverage 84.06% 84.49% +0.42% ========================================== Files 14 14 Lines 1268 1238 -30 Branches 249 245 -4 ========================================== - Hits 1066 1046 -20 + Misses 155 149 -6 + Partials 47 43 -4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

benoit74 commented 3 months ago

CodeFactor is complaining about files which should be ignored since they are outside our scope, these are sample files from online websites.

rgaudin commented 3 months ago

Can you add exceptions then? Either inline, via a config file or via Codefactor UI

benoit74 commented 3 months ago

Can we rename guessed_charsets with charsets_to_try ?

Sure

Good luck with Codefactor 😀

I will kill it ^^ It worked once, I supposed my last modification was ok so I cleaned up everything I've left over, it's not working anymore, I rolled-back to what was working, it is not working anymore. But I will nail it ^^

benoit74 commented 3 months ago

Codefactor issues its configuration from main branch, so you have to merge first to main before you can review the PR ...

rgaudin commented 3 months ago

Go ahead

kelson42 commented 3 months ago

@benoit74 Bravo... and bon courage!

benoit74 commented 3 months ago

Go ahead

I already pushed only the configuration to main branch. My comment was more for "the posterity". I still have to rewrite this branch to change arg name + simplify commits.

Bravo... and bon courage!

We will finish by nailing this down ^^