Skip to content

Commit a1acc50

Browse files
Copilotdmichon-msft
andcommitted
Refactor for better readability per review feedback
- Rename shouldIncludeProjectAsync (return bool vs void) - Pass isDiffVersionOnlyChange function as parameter - Remove "Helper" from isVersionOnlyChangeAsync name - Caller adds to changedProjects based on return value Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
1 parent 4ac21b9 commit a1acc50

1 file changed

Lines changed: 33 additions & 28 deletions

File tree

libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ export class ProjectChangeAnalyzer {
122122

123123
const changedProjects: Set<RushConfigurationProject> = new Set();
124124

125+
// Arrow function that references this._git for version-only change checking
126+
const isDiffVersionOnlyChange = async (diffStatus: IFileDiffStatus): Promise<boolean> => {
127+
return await isVersionOnlyChangeAsync(diffStatus, repoRoot, this._git);
128+
};
129+
125130
if (enableFiltering) {
126131
// Reading rush-project.json may be problematic if, e.g. rush install has not yet occurred and rigs are in use
127132
await Async.forEachAsync(
@@ -134,31 +139,33 @@ export class ProjectChangeAnalyzer {
134139
terminal
135140
);
136141

137-
await checkAndAddProjectAsync(
142+
const shouldInclude: boolean = await shouldIncludeProjectAsync(
138143
project,
139144
filteredChanges,
140145
excludeVersionOnlyChanges,
141146
lookup,
142-
repoRoot,
143-
this._git,
144-
changedProjects
147+
isDiffVersionOnlyChange
145148
);
149+
if (shouldInclude) {
150+
changedProjects.add(project);
151+
}
146152
},
147153
{ concurrency: 10 }
148154
);
149155
} else {
150156
await Async.forEachAsync(
151157
changesByProject,
152158
async ([project, projectChanges]) => {
153-
await checkAndAddProjectAsync(
159+
const shouldInclude: boolean = await shouldIncludeProjectAsync(
154160
project,
155161
projectChanges,
156162
excludeVersionOnlyChanges,
157163
lookup,
158-
repoRoot,
159-
this._git,
160-
changedProjects
164+
isDiffVersionOnlyChange
161165
);
166+
if (shouldInclude) {
167+
changedProjects.add(project);
168+
}
162169
},
163170
{ concurrency: 10 }
164171
);
@@ -480,27 +487,24 @@ export class ProjectChangeAnalyzer {
480487
}
481488

482489
/**
483-
* Helper function to check if a project should be added to the changed projects set,
484-
* optionally filtering out version-only and changelog changes.
490+
* Checks if a project should be considered changed, optionally filtering out version-only and changelog changes.
491+
* @returns true if the project has substantive changes, false if changes should be excluded
485492
*/
486-
async function checkAndAddProjectAsync(
493+
async function shouldIncludeProjectAsync(
487494
project: RushConfigurationProject,
488495
projectChanges: Map<string, IFileDiffStatus>,
489496
excludeVersionOnlyChanges: boolean | undefined,
490497
lookup: LookupByPath<RushConfigurationProject>,
491-
repoRoot: string,
492-
git: Git,
493-
changedProjects: Set<RushConfigurationProject>
494-
): Promise<void> {
495-
// Early return if no changes
498+
isDiffVersionOnlyChange: (diffStatus: IFileDiffStatus) => Promise<boolean>
499+
): Promise<boolean> {
500+
// No changes means don't include
496501
if (projectChanges.size === 0) {
497-
return;
502+
return false;
498503
}
499504

500-
// If excludeVersionOnlyChanges is not enabled, add the project
505+
// If excludeVersionOnlyChanges is not enabled, include the project
501506
if (!excludeVersionOnlyChanges) {
502-
changedProjects.add(project);
503-
return;
507+
return true;
504508
}
505509

506510
// Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json
@@ -509,8 +513,7 @@ async function checkAndAddProjectAsync(
509513
const match: IPrefixMatch<RushConfigurationProject> | undefined = lookup.findLongestPrefixMatch(filePath);
510514
if (!match) {
511515
// This should be unreachable as projectChanges contains files where match.value === project
512-
changedProjects.add(project);
513-
return;
516+
return true;
514517
}
515518

516519
const projectRelativePath: string = filePath.slice(match.index);
@@ -522,22 +525,24 @@ async function checkAndAddProjectAsync(
522525

523526
// Check if this is package.json at project root with version-only changes
524527
if (projectRelativePath === '/package.json') {
525-
const isVersionOnlyChange: boolean = await isVersionOnlyChangeHelperAsync(diffStatus, repoRoot, git);
528+
const isVersionOnlyChange: boolean = await isDiffVersionOnlyChange(diffStatus);
526529
if (isVersionOnlyChange) {
527530
continue; // Skip version-only package.json changes
528531
}
529532
}
530533

531-
// Found a non-excluded change, add the project
532-
changedProjects.add(project);
533-
return;
534+
// Found a non-excluded change
535+
return true;
534536
}
537+
538+
// All changes were excluded
539+
return false;
535540
}
536541

537542
/**
538-
* Helper function to check if a diff represents a version-only change to package.json.
543+
* Checks if a diff represents a version-only change to package.json.
539544
*/
540-
async function isVersionOnlyChangeHelperAsync(
545+
async function isVersionOnlyChangeAsync(
541546
diffStatus: IFileDiffStatus,
542547
repoRoot: string,
543548
git: Git

0 commit comments

Comments
 (0)