Batch Git config reads for branch operations - #8978
Open
Tamir Duberstein (tamird) wants to merge 2 commits into
Open
Tamir Duberstein (tamird) wants to merge 2 commits into
Tamir Duberstein (tamird) wants to merge 2 commits into
Conversation
Copilot started reviewing on behalf of
Tamir Duberstein (tamird)
September 22, 2026 02:38
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Optimizes local pull request refreshes by reading repository configuration once per refresh.
Changes:
- Adds batch metadata lookup for multiple branches.
- Reuses one configuration snapshot across branch chunks.
- Expands tests for batching, refresh behavior, and failures.
| File | Description |
|---|---|
src/test/github/pullRequestGitHelper.test.ts |
Expands duplicate metadata coverage. |
src/test/github/folderRepositoryManager.test.ts |
Tests configuration snapshot behavior. |
src/github/pullRequestGitHelper.ts |
Implements batch metadata lookup. |
src/github/folderRepositoryManager.ts |
Reuses metadata during local PR discovery. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Tamir Duberstein (tamird)
changed the title
Copilot started reviewing on behalf of
Tamir Duberstein (tamird)
September 22, 2026 02:55
View session
Alex Ross (alexr00)
left a comment
Alex Ross (alexr00)
left a comment
Member
There was a problem hiding this comment.
Looks reasonable, thank you! One question, and there are conflicts.
Local PR enumeration queried all Git config for every branch. Match all requested branches in one helper-owned config read and one pass, keeping the highest finite PR number and the first match on ties. Single-branch lookups use the same helper; each enumeration reads fresh config.
First-activation branch association and branch deletion still reread repository config for each branch. Read metadata once for the selected branches, and inspect deletion leftovers after each mutation phase. Only refresh the cleanup snapshot when fallback deletions are needed. Keep association cleanup before that refresh so known metadata is removed even if reading the remaining config fails. Remove the single-branch metadata API and its manager wrapper. Callers use the batch API even when they need only one branch, keeping each operation responsible for the set of branches it requests. Normalize folderRepositoryManager.ts line endings to LF.
Tamir Duberstein (tamird)
force-pushed
the
perf/local-pr-config-snapshot
branch
from
September 30, 2026 00:20
034c570 to
eae1d7b
Compare
Copilot started reviewing on behalf of
Tamir Duberstein (tamird)
September 30, 2026 00:20
View session
Alex Ross (alexr00)
left a comment
Alex Ross (alexr00)
left a comment
Member
There was a problem hiding this comment.
Tamir Duberstein (@tamird), just for the future: it's hard to re-review PRs that are force pushed. Instead of just reviewing your new changes, I have to re-review everything.
Contributor
Author
|
Alex Ross (@alexr00) ack, will try to avoid that in the future. FWIW this is why I use reviewable.io for OSS projects I maintain. 20 years in GH code review is still extremely basic. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Local pull request discovery reads the complete Git configuration separately for every local branch. Reading one snapshot and indexing the requested branches reduces this from N config reads to one: for 102 branches, 102 reads become 1, eliminating 101 reads (about 99%). The config entries are scanned once instead of once per branch.
Use the same batch lookup for first-activation association, reducing metadata reads from up to 10 to 1. Branch deletion reads config once after the deletion batch and refreshes it only when fallback deletions were needed, replacing the per-branch reads while preserving cleanup of leftover settings.
Remove the single-branch metadata API and migrate its callers to the batch API. Keep the Map for branch lookup and presence checks, preserve the highest PR number and the first entry on ties, and retain the regression covering reuse across branch chunks and fresh data on the next refresh. Also normalize folderRepositoryManager.ts to LF.