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

Issue 155: log metadata prior to verification (PR #160 fixes) #172

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

rgaudin
Copy link
Member

@rgaudin rgaudin commented Jun 20, 2024

I can't push on #160 so here's a copy of it with my fixes at the end.
Should it be satisfactory, I suggest we squash all commits into one.

Fixes #155

Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it is indeed way simpler to read for maintainers. Less open to later modifications, but as already said this would be premature optimization from my PoV.

I just have one small question about one log message from not decodable metadata, I feel like we should be more explicit.

src/zimscraperlib/zim/creator.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (ada7f73) to head (def563a).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #172   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           32        32           
  Lines         1412      1439   +27     
  Branches       243       248    +5     
=========================================
+ Hits          1412      1439   +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rgaudin rgaudin requested a review from benoit74 June 21, 2024 10:25
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I just squashed the commits and edited the commit message. Commit is still attributed to @richterdavid which is ok.

@benoit74 benoit74 merged commit f888743 into main Jun 21, 2024
10 checks passed
@benoit74 benoit74 deleted the PR160fixes branch June 21, 2024 11:15
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.

Display all metadata in debug log level
2 participants