-
Notifications
You must be signed in to change notification settings - Fork 51
Feat/git config native #407
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
Feat/git config native #407
Conversation
add tests to native git config implementation
svarlamov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR had a few thoughts:
git configworks even outside of a repo. I noticed that you're passing a filepath into the repo -- what if the user is outside of a repo? Will this still work?- Please use the existing TestRepo harness instead of creating another utility for test repos for the unit tests. You can reference the existing unit tests in the codebase to see how to use TestRepo
- Noticed the tests use the serial macro. We try to avoid using it if at all possible -- with TestRepo maybe we could avoid it?
|
Hey @svarlamov ,
|
|
about current git config implementation:
|
svarlamov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I added some tests for bare clones, etc. but it was pretty complete. Thanks!
Fixes #376
git configcommands invocations with gix-config native Rust implementationGixErrortypegit configcommands