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

cwd consistency #611

Merged
merged 4 commits into from Nov 20, 2022
Merged

cwd consistency #611

merged 4 commits into from Nov 20, 2022

Conversation

Byron
Copy link
Owner

@Byron Byron commented Nov 20, 2022

Make sure our usage of the current_dir is somewhat consistent (#607)

With this patch set it's used implicitly only where absolutely required, namely git-odb in the
Store and in the higher-level git-repository.

…and returns `Option<path>`

Previously the function was willing to return an empty path despite it
being invalid. With the `current_dir` being required, this won't be the
case anymore and will yield logically consistent results in all cases.

This forces the caller to deal with the relative path being invalid
or crafted to produce some other path, maybe to bypass sanity checks.
@Byron Byron force-pushed the cwd-consistency branch 2 times, most recently from ddfdcc1 to 143c417 Compare November 20, 2022 15:34
…ment and returns `Option<path>`

That way it's possible to avoid at least one call of
`std::env::current_dir()` per invocation, which also is more consnstent
with similar plumbing methods.

Furthermore it can signal with `None` if the input directory was invalid.
That way it's more consistent with similar low-level functions and it's
possible to avoid multiple calls to `std::env::current_dir()`.

Furthermore, the usage of `current_dir()` is made explicit when
instantiating a store to allow it to be resued.
@Byron Byron merged commit ea7c6a3 into main Nov 20, 2022
@Byron Byron deleted the cwd-consistency branch November 22, 2022 07:21
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.

None yet

1 participant