Skip to content

Commit cc7997f

Browse files
Copilotalexr00
andcommitted
Address code review feedback: improve variable names and readability
- Renamed hasExplicitRemotesConfig to hasUserConfiguredRemotes for clarity - Extracted complex conditional logic into well-named boolean variables (hasReceivedData, isFetchingNextPage, hasReachedPreviousFetchLimit, shouldBreakEarly) to improve readability and make the logic more self-documenting Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 6482dc8 commit cc7997f

File tree

1 file changed

+8
-8
lines changed

1 file changed

+8
-8
lines changed

src/github/folderRepositoryManager.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,7 +1088,7 @@ export class FolderRepositoryManager extends Disposable {
10881088

10891089
// Check if user has explicitly configured remotes (not using defaults)
10901090
const remotesConfig = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).inspect<string[]>(REMOTES);
1091-
const hasExplicitRemotesConfig = !!(remotesConfig?.globalValue || remotesConfig?.workspaceValue || remotesConfig?.workspaceFolderValue);
1091+
const hasUserConfiguredRemotes = !!(remotesConfig?.globalValue || remotesConfig?.workspaceValue || remotesConfig?.workspaceFolderValue);
10921092

10931093
const githubRepositories = this._githubRepositories.filter(repo => {
10941094
if (!activeGitHubRemotes.find(r => r.equals(repo.remote))) {
@@ -1152,17 +1152,17 @@ export class FolderRepositoryManager extends Disposable {
11521152

11531153
pageInformation.hasMorePages = itemData.hasMorePages;
11541154

1155-
// Break early if
1155+
// Determine if we should break early from the loop:
11561156
// 1) we've received data AND
11571157
// 2) either we're fetching just the next page (case 2)
11581158
// OR we're fetching all (cases 1&3), and we've fetched as far as we had previously (or further, in case 1).
11591159
// 3) AND the user hasn't explicitly configured remotes (if they have, we should search all of them)
1160-
if (
1161-
itemData.items.length &&
1162-
(options.fetchNextPage ||
1163-
((options.fetchNextPage === false) && !options.fetchOnePagePerRepo && (pagesFetched >= getTotalFetchedPages()))) &&
1164-
!hasExplicitRemotesConfig
1165-
) {
1160+
const hasReceivedData = itemData.items.length > 0;
1161+
const isFetchingNextPage = options.fetchNextPage;
1162+
const hasReachedPreviousFetchLimit = (options.fetchNextPage === false) && !options.fetchOnePagePerRepo && (pagesFetched >= getTotalFetchedPages());
1163+
const shouldBreakEarly = hasReceivedData && (isFetchingNextPage || hasReachedPreviousFetchLimit) && !hasUserConfiguredRemotes;
1164+
1165+
if (shouldBreakEarly) {
11661166
if (getTotalFetchedPages() === 0) {
11671167
// We're in case 1, manually set number of pages we looked through until we found first results.
11681168
setTotalFetchedPages(pagesFetched);

0 commit comments

Comments
 (0)