Skip to content

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.

@codecov
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
@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