feat: resolve git identities from command line if supplied#199
feat: resolve git identities from command line if supplied#199saintedlama wants to merge 6 commits intomainfrom
Conversation
Pull Request Test Coverage Report for Build 22274036918Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
Adds support for overriding Git commit/tag identity via CLI options, to avoid failures when LibGit2Sharp can’t resolve user.name / user.email from certain Git config setups (e.g., includeIf via env vars), and wires that identity into release commit/tag creation and validation.
Changes:
- Introduce
IGitIdentityResolver/GitIdentityResolverand register it in DI. - Use the resolver for identity validation and for author/tagger signatures; pass identity overrides to
gitfor signed commit/tag operations. - Add
--git-user-name/--git-user-emailCLI options, update docs, and add/adjust tests.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Versionize/Program.cs | Registers IGitIdentityResolver in the DI container. |
| Versionize/Lifecycle/ReleaseTagger.cs | Uses resolver for tagger signature and for signed-tag git -c arguments. |
| Versionize/Lifecycle/ReleaseCommitter.cs | Uses resolver for author signature and for signed-commit git -c arguments. |
| Versionize/Git/RepoStateValidator.cs | Delegates git-identity validation to IGitIdentityResolver. |
| Versionize/Git/GitProcessUtil.cs | Extends signed commit/tag helpers to accept extra git config args. |
| Versionize/Git/GitIdentityResolver.cs | New resolver that reads CLI overrides first, then falls back to repo config. |
| Versionize/Config/VersionizeOptions.cs | Adds GitUserName / GitUserEmail to merged runtime options. |
| Versionize/Config/ConfigProvider.cs | Maps new CLI options into VersionizeOptions. |
| Versionize/Config/CliConfig.cs | Adds --git-user-name / --git-user-email options. |
| Versionize/CommandLine/ErrorMessages.cs | Updates missing-git-config guidance to include new CLI overrides. |
| Versionize.Tests/TestSupport/GitIdentityResolverTestHelper.cs | Adds test helper for creating a resolver with identity overrides. |
| Versionize.Tests/Lifecycle/ReleaseTaggerTests.cs | Updates construction for DI change; adds override coverage for tagger identity. |
| Versionize.Tests/Lifecycle/ChangeCommitterTests.cs | Updates construction for DI change; adds override coverage for committer identity. |
| Versionize.Tests/Git/RepoStateValidatorTests.cs | Updates construction for DI change; adds override coverage for validator. |
| Versionize.Tests/FunctionalTests/Program.VersionizeCommandTests.cs | Adds functional coverage for CLI identity overrides; removes prior config-extends functional tests. |
| Versionize.Tests/Config/ConfigProviderTests.cs | Adds tests asserting new CLI options flow into VersionizeOptions. |
| README.md | Documents the new CLI options. |
…ouldExtendConfigurationFromFile tests
|
@cabauman I think it fits quite nice in the existing architecture and adds a level of indirection that could be usable in the future |
|
That sounds like a welcome feature to me! I'll take a look at the changes tomorrow. You're on a roll 💪 |
|
And by the way, feel free to trigger the publish pipeline after we get everything in. I made it manual in order to have more control. |
|
Kicking off some agents and then verifying the outcome is honestly so much fun. And versionize is perfect for this with a clear codebase and awesome written issues |
|
Right! |
| return new Signature(identity.UserName!, identity.UserEmail!, now); | ||
| } | ||
|
|
||
| private static string BuildGitConfigArguments(GitIdentity identity) |
There was a problem hiding this comment.
Since ReleaseTagger and ReleaseCommitter use the same helpers, should we consider moving BuildGitConfigArguments and EscapeGitConfigValue to GitProcessUtil?
| [Fact] | ||
| public void ShouldExtendConfigurationFromFile() | ||
| { | ||
| // Arrange |
There was a problem hiding this comment.
Did Copilot remove these comments from unrelated tests? Because many of our tests have verbose setup and numerous asserts, I like having these comments. But if you dislike them, we can consider removing all the comments in a dedicated PR.
Closes #187