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

fix: dragonflyoss#1644 and #1651 resolve Algorithm to_string and FromStr inconsistency #1656

Conversation

cslinwang
Copy link
Contributor

Relevant Issue (if applicable)

Fixes: #1644
Fixes: #1651

Details

This Pull Request addresses the inconsistency between the to_string and FromStr implementations for the Algorithm enum.
Previously:

  • The to_string method generated PascalCase outputs (e.g., Zstd).
  • The FromStr implementation expected lowercase strings (e.g., zstd).
    This caused parsing issues where the output of to_string could not be used as input for FromStr.

Changes:

  1. Refactored the Display trait for the Algorithm enum to ensure to_string produces lowercase strings matching FromStr.
  2. Added unit tests to validate consistency and handle invalid inputs.

Types of changes

What types of changes does your PullRequest introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

Go over all the following points, and put an x in all the boxes that apply.

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@cslinwang cslinwang requested a review from a team as a code owner December 16, 2024 11:26
@cslinwang cslinwang requested review from hsiangkao, power-more and Desiki-high and removed request for a team December 16, 2024 11:26
…ing and FromStr inconsistency

Signed-off-by: Lin Wang <[email protected]>
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

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

Project coverage is 60.01%. Comparing base (acdf021) to head (e22b202).

Files with missing lines Patch % Lines
utils/src/compress/mod.rs 97.61% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1656      +/-   ##
==========================================
- Coverage   60.02%   60.01%   -0.01%     
==========================================
  Files         147      147              
  Lines       49583    49624      +41     
  Branches    47037    47078      +41     
==========================================
+ Hits        29761    29784      +23     
- Misses      18041    18056      +15     
- Partials     1781     1784       +3     
Files with missing lines Coverage Δ
utils/src/compress/mod.rs 97.14% <97.61%> (+0.04%) ⬆️

... and 5 files with indirect coverage changes

Copy link
Collaborator

@imeoer imeoer left a comment

Choose a reason for hiding this comment

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

LGTM!

@imeoer imeoer merged commit e23d5bc into dragonflyoss:master Dec 16, 2024
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants