[pull] master from microsoft:master - #145
Merged
Merged
Conversation
Add LibGit2Repo.GetConfigBoolOrDefault(...) (instance + static overloads) for one-off boolean config reads, replacing scattered short-lived LibGit2Repo/LibGit2RepoInvoker usage at 4 call sites: - GVFS/CommandLine/CloneVerb.cs (gvfs.trust-pack-indexes) - GVFS.Hooks/Program.cs (gvfs.show-hydration-status) - GVFS.Mount/InProcessMount.cs (gvfs.background-cache-auth) - GVFS/CommandLine/PrefetchVerb.cs (gvfs.prefetch-offload) LibGit2RepoInvoker.InitializeSharedRepo() intentionally forces an object-store probe so long-lived/shared callers can amortize object-store load costs. That is wasted work for one-off config reads that immediately dispose the repo. The helper methods live directly on LibGit2Repo rather than a separate extension class, matching the repo.GetConfigBoolOrDefault(name, default) convention already documented in AGENTS.md, and avoiding unnecessary indirection for a class the team owns in the same assembly. Both methods fall back to defaultValue and log a RelatedWarning on any failure, matching the "default on any failure" contract each call site previously implemented independently. Added a protected LibGit2Repo(ITracer tracer) constructor to support test doubles that inject a mock tracer without opening a real repo. Surveyed master for other short-lived config-only LibGit2Repo/ LibGit2RepoInvoker usage; PrefetchStep.cs, GitStatusCache.cs, and GitRepo.cs were left alone because they use shared/long-lived repo access, not the transient anti-pattern this change addresses. Reviewed with an internal 6-lens review-swarm pass (correctness, security, design, tests, async-parallelism, risk-rollout); addressed all actionable findings: - Widened the shared helper's exception handling to a plain catch (Exception), restoring the "default on any failure" guarantee InProcessMount/PrefetchVerb relied on before this refactor. - Fixed a double-RelatedWarning log on the repo-open-failure path. - Replaced a hardcoded, non-portable "Z:\..." path in a unit test with a GUID-suffixed temp path. - Added test coverage for the unset-key (null-coalescing) branch and the InvalidDataException catch arm. - Simplified the parameterless constructor to delegate to the tracer-accepting one. Full unit test suite: 891 passed, 0 failed, 11 skipped (pre-existing, unrelated). Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…ookup Share transient libgit2 config lookup helper
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )