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

rewrite some checksum related code #443

Merged
merged 9 commits into from
Aug 15, 2024
Merged

rewrite some checksum related code #443

merged 9 commits into from
Aug 15, 2024

Conversation

TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Jun 21, 2024

Issue #, if available:

  • Get into this rabbit hole from removing body_user_callback_after_checksum to support write into file directly

Description of changes:

  • No functionality change, beside I modified the request response header directly instead of making a copy.
  • Remove the *_after_checksum callbacks, just invoke the right function from the right place
  • we don't need to make a copy of headers before we send it to the user, we are not using the response headers anymore afterwards
  • reuse some code instead of copy/paste

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@TingDaoK TingDaoK marked this pull request as ready for review June 21, 2024 23:07
@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 88.17204% with 11 lines in your changes missing coverage. Please review.

Project coverage is 89.44%. Comparing base (94e3342) to head (6698fc9).

Files Patch % Lines
source/s3_meta_request.c 89.79% 5 Missing ⚠️
source/s3_util.c 88.46% 3 Missing ⚠️
source/s3_auto_ranged_get.c 66.66% 2 Missing ⚠️
source/s3_default_meta_request.c 88.88% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
- Coverage   89.62%   89.44%   -0.19%     
==========================================
  Files          20       20              
  Lines        6092     6080      -12     
==========================================
- Hits         5460     5438      -22     
- Misses        632      642      +10     
Files Coverage Δ
source/s3_auto_ranged_put.c 92.78% <100.00%> (-0.02%) ⬇️
source/s3_default_meta_request.c 95.23% <88.88%> (-0.52%) ⬇️
source/s3_auto_ranged_get.c 97.23% <66.66%> (-0.55%) ⬇️
source/s3_util.c 97.16% <88.46%> (-0.78%) ⬇️
source/s3_meta_request.c 92.77% <89.79%> (-0.69%) ⬇️

@TingDaoK TingDaoK merged commit 3d821fe into main Aug 15, 2024
31 checks passed
@TingDaoK TingDaoK deleted the checksum-callback branch August 15, 2024 22:12
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.

4 participants