Skip to content

Commit 50b9ee3

Browse files
committed
Resolve CodeReview comments for:
1. libraries/rush-lib/src/logic/operations/WeightedOperationPlugin.ts - Added more comments to explain the percentage weight convert function. - Use `as` syntax to cast error to Error type for better readability. 2. libraries/rush-lib/src/schemas/rush-project.schema.json - Updated the description of the weight property to reflect that the maximum concurrency units is determined by `os.availableParallelism()`. 3. libraries/rush-lib/src/logic/operations/test/WeightedOperationPlugin.test.ts - Fix a test name.
1 parent 02dea2d commit 50b9ee3

3 files changed

Lines changed: 14 additions & 5 deletions

File tree

libraries/rush-lib/src/logic/operations/WeightedOperationPlugin.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,15 @@ function weightOperations(
3737

3838
const percentageRegExp: RegExp = /^[1-9][0-9]*(\.\d+)?%$/;
3939

40+
/**
41+
* Since the JSON value is a string, it must be a percentage like "50%",
42+
* which we convert to a number based on the available parallelism.
43+
* For example, if the available parallelism (not the -p flag) is 8 and the weight is "50%",
44+
* then the resulting weight will be 4.
45+
*
46+
* @param weight
47+
* @returns
48+
*/
4049
function _tryConvertPercentWeight(weight: `${number}%`): number {
4150
if (!percentageRegExp.test(weight)) {
4251
throw new Error(`Expected a percentage string like "100%".`);
@@ -65,7 +74,7 @@ function weightOperations(
6574
operation.weight = _tryConvertPercentWeight(operationSettings.weight);
6675
} catch (error) {
6776
throw new Error(
68-
`${operation.name} (invalid weight: ${operationSettings.weight}) ${error instanceof Error ? error.message : String(error)}`
77+
`${operation.name} (invalid weight: ${JSON.stringify(operationSettings.weight)}) ${(error as Error).message}`
6978
);
7079
}
7180
}

libraries/rush-lib/src/logic/operations/test/WeightedOperationPlugin.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,10 @@ describe(WeightedOperationPlugin.name, () => {
149149
expect(operation.weight).toBe(4);
150150
});
151151

152-
it('use ceiling when converting percentage weight to avoid zero weight', async () => {
152+
it('use floor when converting percentage weight to avoid zero weight', async () => {
153153
mockAvailableParallelism(16);
154154

155-
const project: RushConfigurationProject = createProject('project-ceiling');
155+
const project: RushConfigurationProject = createProject('project-floor');
156156
const operation: Operation = createOperation({
157157
project,
158158
settings: {
@@ -197,6 +197,6 @@ describe(WeightedOperationPlugin.name, () => {
197197

198198
await expect(
199199
applyWeightPluginAsync(createExecutionRecords(operation), createContext(new Map()))
200-
).rejects.toThrow(/invalid weight: 12.5a%/i);
200+
).rejects.toThrow(/invalid weight: "12.5a%"/i);
201201
});
202202
});

libraries/rush-lib/src/schemas/rush-project.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@
110110
{
111111
"type": "string",
112112
"pattern": "^[1-9][0-9]*(\\.\\d+)?%$",
113-
"description": "The percentage of concurrency units that this operation should take up. The maximum concurrency units is determined by the -p flag."
113+
"description": "The percentage of concurrency units that this operation should take up. The maximum concurrency units is determined by `os.availableParallelism()`."
114114
},
115115
{
116116
"description": "The number of concurrency units that this operation should take up. The maximum concurrency units is determined by the -p flag.",

0 commit comments

Comments
 (0)