Skip to content

Conversation

@bernoussama
Copy link
Contributor

@bernoussama bernoussama commented Jan 27, 2026

Fixes #376

  • Replaced git config commands invocations with gix-config native Rust implementation
  • Added GixError type
  • Added gix_config tests, ensuring same output as git config commands

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@svarlamov svarlamov left a 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:

  1. git config works 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?
  2. 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
  3. Noticed the tests use the serial macro. We try to avoid using it if at all possible -- with TestRepo maybe we could avoid it?

@bernoussama
Copy link
Contributor Author

Hey @svarlamov ,

  1. Yes it should work if not inside a repo it'll fallback to env and global config as per gix_config docs, but it should be tested, will include a test for it, good observation.
  2. I will look into TestRepo and use it for testing
  3. For the serial macro, I'll use TestRepo for tests and see how it goes

@bernoussama bernoussama requested a review from svarlamov January 27, 2026 23:38
@bernoussama
Copy link
Contributor Author

about current git config implementation:

  • It'll work outside of a repo, as i added a fallback to global Git config file if not inside a Git repo. This scenario remains untested however, because it would involve modifying env variables requiring these tests to be serial.

@svarlamov svarlamov self-assigned this Jan 28, 2026
Copy link
Member

@svarlamov svarlamov left a 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!

@svarlamov svarlamov merged commit f25cc01 into git-ai-project:main Jan 28, 2026
5 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.

Git config native implementation

3 participants