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

Rename assert_eq_path and assert_matches_path in snapbox #225

Closed
not-my-profile opened this issue Aug 18, 2023 · 3 comments
Closed

Rename assert_eq_path and assert_matches_path in snapbox #225

not-my-profile opened this issue Aug 18, 2023 · 3 comments
Labels
A-snapbox Area: snapbox package

Comments

@not-my-profile
Copy link

not-my-profile commented Aug 18, 2023

I consider the current names to be highly confusing. They don't assert that a path is equal or that something matches a path ... I think it would make sense to rename them to assert_file_eq and assert_file_matches.

This would be a breaking change but I think well worth it.

@not-my-profile not-my-profile changed the title Rename assert_*_path methods to assert_*_file in snapbox Rename assert_*_path functions to assert_*_file in snapbox Aug 18, 2023
@not-my-profile not-my-profile changed the title Rename assert_*_path functions to assert_*_file in snapbox Rename assert_eq_path and assert_matches_path in snapbox Aug 18, 2023
@epage
Copy link
Contributor

epage commented Aug 18, 2023

I consider this one of those "no win" situations with naming. However, I expect this problem to go away with #221 because we'll instead say assert_eq!(Data::path(path), ...)

Because of that, I think I'll go ahead and close this

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Aug 18, 2023
@epage epage added the A-snapbox Area: snapbox package label Aug 18, 2023
@not-my-profile
Copy link
Author

Did you mean to link #221? I'm somewhat confused how that issue relates to assert_eq!(Data::path(path), ...).

@not-my-profile
Copy link
Author

#227 (comment)

I expect #221 to change up the API in way that will make this clearer.

Ah ok ... nevermind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-snapbox Area: snapbox package
Projects
None yet
Development

No branches or pull requests

2 participants