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

use error type for A2lFile.write() as well #25

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Conversation

tardyp
Copy link
Contributor

@tardyp tardyp commented Nov 14, 2023

following up the change in 1.4.0, this is a conversion to thiserror for write() api.

This is a breaking change.

I also changed a bit the other part of the error handling to be consistent in the use of map_err

with 1.4.0 the parse api is now returning error.
Fix the api to have this for write as well.

this allows the client to just call write(..)? in a main function

Signed-off-by: Pierre Tardy <[email protected]>
in order to be more consistent, I simplified the code to use map_err everywhere

map_err beeing the rust idiomatic way to convert errors, compared to pattern matching

Signed-off-by: Pierre Tardy <[email protected]>
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (17111a6) 85.63% compared to head (26d8d3e) 85.60%.

Files Patch % Lines
src/lib.rs 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   85.63%   85.60%   -0.03%     
==========================================
  Files          27       27              
  Lines       10199    10193       -6     
==========================================
- Hits         8734     8726       -8     
- Misses       1465     1467       +2     

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

@DanielT
Copy link
Owner

DanielT commented Nov 14, 2023

I like it. Thanks!

@DanielT DanielT merged commit b5e0fd9 into DanielT:main Nov 14, 2023
3 of 4 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
Development

Successfully merging this pull request may close these issues.

2 participants