Skip to content

Uses Into<PathBuf> for path in AppendVec::new()#433

Merged
brooksprumo merged 1 commit intoanza-xyz:masterfrom
brooksprumo:paths/append-vec
Mar 26, 2024
Merged

Uses Into<PathBuf> for path in AppendVec::new()#433
brooksprumo merged 1 commit intoanza-xyz:masterfrom
brooksprumo:paths/append-vec

Conversation

@brooksprumo
Copy link
Copy Markdown

Problem

While reviewing #334, I made a wrong suggestion and noticed that the path parameter in AppendVec::new() et al should be Into<PathBuf> instead of a raw Path or AsRef<Path>.

Summary of Changes

Update AppendVec::new() et al to take the path parameter as Into<PathBuf>, and update callers.

@brooksprumo brooksprumo marked this pull request as ready for review March 26, 2024 16:38
Copy link
Copy Markdown

@yhchiang-sol yhchiang-sol left a comment

Choose a reason for hiding this comment

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

LG!

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (30eecd6) to head (29f0a3c).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #433     +/-   ##
=========================================
- Coverage    81.9%    81.8%   -0.1%     
=========================================
  Files         840      841      +1     
  Lines      228107   228253    +146     
=========================================
+ Hits       186851   186919     +68     
- Misses      41256    41334     +78     

Copy link
Copy Markdown

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm

@brooksprumo brooksprumo merged commit 21b6821 into anza-xyz:master Mar 26, 2024
@brooksprumo brooksprumo deleted the paths/append-vec branch March 26, 2024 18:44
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