Skip to content

Commit df43c85

Browse files
Copilotdmichon-msft
Copilot
and
dmichon-msft
committed
Address PR feedback: use ignoredParameterValues and improve filtering
Co-authored-by: dmichon-msft <[email protected]>
1 parent 873d74e commit df43c85

File tree

7 files changed

+74
-44
lines changed

7 files changed

+74
-44
lines changed

‎libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export interface IIPCOperationRunnerOptions {
3030
commandForHash: string;
3131
persist: boolean;
3232
requestRun: OperationRequestRunCallback;
33-
ignoredParameterNames: ReadonlyArray<string>;
33+
ignoredParameterValues: ReadonlyArray<string>;
3434
}
3535

3636
function isAfterExecuteEventMessage(message: unknown): message is IAfterExecuteEventMessage {
@@ -60,7 +60,7 @@ export class IPCOperationRunner implements IOperationRunner {
6060
private readonly _commandForHash: string;
6161
private readonly _persist: boolean;
6262
private readonly _requestRun: OperationRequestRunCallback;
63-
private readonly _ignoredParameterNames: ReadonlyArray<string>;
63+
private readonly _ignoredParameterValues: ReadonlyArray<string>;
6464

6565
private _ipcProcess: ChildProcess | undefined;
6666
private _processReadyPromise: Promise<void> | undefined;
@@ -77,7 +77,7 @@ export class IPCOperationRunner implements IOperationRunner {
7777

7878
this._persist = options.persist;
7979
this._requestRun = options.requestRun;
80-
this._ignoredParameterNames = options.ignoredParameterNames;
80+
this._ignoredParameterValues = options.ignoredParameterValues;
8181
}
8282

8383
public async executeAsync(context: IOperationRunnerContext): Promise<OperationStatus> {
@@ -89,8 +89,8 @@ export class IPCOperationRunner implements IOperationRunner {
8989
terminal.writeLine('Invoking: ' + this._commandToRun);
9090

9191
// Log any ignored parameters in verbose mode
92-
if (this._ignoredParameterNames.length > 0) {
93-
terminal.writeVerboseLine(`Ignored parameters: ${this._ignoredParameterNames.join(', ')}`);
92+
if (this._ignoredParameterValues.length > 0) {
93+
terminal.writeVerboseLine(`Ignored parameters: ${this._ignoredParameterValues.join(' ')}`);
9494
}
9595

9696
const { rushConfiguration, projectFolder } = this._rushProject;

‎libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { OperationStatus } from './OperationStatus';
1313
import {
1414
PLUGIN_NAME as ShellOperationPluginName,
1515
formatCommand,
16-
getCustomParameterValuesForOperation,
16+
getCustomParameterValuesByOperation,
1717
type ICustomParameterValuesForOperation,
1818
getDisplayName
1919
} from './ShellOperationRunnerPlugin';
@@ -46,7 +46,7 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin {
4646
currentContext = context;
4747

4848
const getCustomParameterValues: (operation: Operation) => ICustomParameterValuesForOperation =
49-
getCustomParameterValuesForOperation();
49+
getCustomParameterValuesByOperation();
5050

5151
for (const operation of operations) {
5252
const { associatedPhase: phase, associatedProject: project, runner } = operation;
@@ -73,7 +73,7 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin {
7373
// for this operation (or downstream operations) to be restored from the build cache.
7474
const commandForHash: string | undefined = phase.shellCommand ?? scripts?.[phaseName];
7575

76-
const { parameterValues: customParameterValues, ignoredParameterNames } =
76+
const { parameterValues: customParameterValues, ignoredParameterValues } =
7777
getCustomParameterValues(operation);
7878
const commandToRun: string = formatCommand(rawScript, customParameterValues);
7979

@@ -87,7 +87,7 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin {
8787
commandToRun,
8888
commandForHash,
8989
persist: true,
90-
ignoredParameterNames,
90+
ignoredParameterValues,
9191
requestRun: (requestor: string, detail?: string) => {
9292
const operationState: IOperationExecutionResult | undefined =
9393
operationStatesByRunner.get(ipcOperationRunner);

‎libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4-
import type { IPhase } from '../../api/CommandLineConfiguration';
54
import type { IOperationSettings, RushProjectConfiguration } from '../../api/RushProjectConfiguration';
65
import type {
76
ICreateOperationsContext,
@@ -13,7 +12,8 @@ import { NullOperationRunner } from './NullOperationRunner';
1312
import { Operation } from './Operation';
1413
import { OperationStatus } from './OperationStatus';
1514
import {
16-
getCustomParameterValuesByPhase,
15+
getCustomParameterValuesByOperation,
16+
type ICustomParameterValuesForOperation,
1717
getDisplayName,
1818
initializeShellOperationRunner
1919
} from './ShellOperationRunnerPlugin';
@@ -46,8 +46,8 @@ export class ShardedPhasedOperationPlugin implements IPhasedCommandPlugin {
4646
function spliceShards(existingOperations: Set<Operation>, context: ICreateOperationsContext): Set<Operation> {
4747
const { rushConfiguration, projectConfigurations } = context;
4848

49-
const getCustomParameterValuesForPhase: (phase: IPhase) => ReadonlyArray<string> =
50-
getCustomParameterValuesByPhase();
49+
const getCustomParameterValues: (operation: Operation) => ICustomParameterValuesForOperation =
50+
getCustomParameterValuesByOperation();
5151

5252
for (const operation of existingOperations) {
5353
const {
@@ -119,10 +119,12 @@ function spliceShards(existingOperations: Set<Operation>, context: ICreateOperat
119119

120120
const collatorDisplayName: string = `${getDisplayName(phase, project)} - collate`;
121121

122-
const customParameters: readonly string[] = getCustomParameterValuesForPhase(phase);
122+
// Get the custom parameter values for the collator, filtered according to the operation settings
123+
const { parameterValues: customParameterValues, ignoredParameterValues } =
124+
getCustomParameterValues(operation);
123125

124126
const collatorParameters: string[] = [
125-
...customParameters,
127+
...customParameterValues,
126128
`--shard-parent-folder="${parentFolder}"`,
127129
`--shard-count="${shards}"`
128130
];
@@ -138,7 +140,7 @@ function spliceShards(existingOperations: Set<Operation>, context: ICreateOperat
138140
rushConfiguration,
139141
commandToRun,
140142
customParameterValues: collatorParameters,
141-
ignoredParameterNames: []
143+
ignoredParameterValues
142144
});
143145

144146
const shardOperationName: string = `${phase.name}:shard`;
@@ -195,7 +197,7 @@ function spliceShards(existingOperations: Set<Operation>, context: ICreateOperat
195197
);
196198

197199
const shardedParameters: string[] = [
198-
...customParameters,
200+
...customParameterValues,
199201
shardArgument,
200202
outputDirectoryArgumentWithShard
201203
];
@@ -209,7 +211,7 @@ function spliceShards(existingOperations: Set<Operation>, context: ICreateOperat
209211
customParameterValues: shardedParameters,
210212
displayName: shardDisplayName,
211213
rushConfiguration,
212-
ignoredParameterNames: []
214+
ignoredParameterValues
213215
});
214216

215217
shardOperation.addDependency(preShardOperation);

‎libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export interface IShellOperationRunnerOptions {
2020
displayName: string;
2121
commandToRun: string;
2222
commandForHash: string;
23-
ignoredParameterNames: ReadonlyArray<string>;
23+
ignoredParameterValues: ReadonlyArray<string>;
2424
}
2525

2626
/**
@@ -45,7 +45,7 @@ export class ShellOperationRunner implements IOperationRunner {
4545

4646
private readonly _rushProject: RushConfigurationProject;
4747

48-
private readonly _ignoredParameterNames: ReadonlyArray<string>;
48+
private readonly _ignoredParameterValues: ReadonlyArray<string>;
4949

5050
public constructor(options: IShellOperationRunnerOptions) {
5151
const { phase } = options;
@@ -56,7 +56,7 @@ export class ShellOperationRunner implements IOperationRunner {
5656
this._rushProject = options.rushProject;
5757
this.commandToRun = options.commandToRun;
5858
this._commandForHash = options.commandForHash;
59-
this._ignoredParameterNames = options.ignoredParameterNames;
59+
this._ignoredParameterValues = options.ignoredParameterValues;
6060
}
6161

6262
public async executeAsync(context: IOperationRunnerContext): Promise<OperationStatus> {
@@ -80,8 +80,8 @@ export class ShellOperationRunner implements IOperationRunner {
8080
terminal.writeLine(`Invoking: ${this.commandToRun}`);
8181

8282
// Log any ignored parameters in verbose mode
83-
if (this._ignoredParameterNames.length > 0) {
84-
terminal.writeVerboseLine(`Ignored parameters: ${this._ignoredParameterNames.join(', ')}`);
83+
if (this._ignoredParameterValues.length > 0) {
84+
terminal.writeVerboseLine(`Ignored parameters: ${this._ignoredParameterValues.join(' ')}`);
8585
}
8686

8787
const { rushConfiguration, projectFolder } = this._rushProject;

‎libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ export class ShellOperationRunnerPlugin implements IPhasedCommandPlugin {
3232
const { rushConfiguration, isInitial } = context;
3333

3434
const getCustomParameterValues: (operation: Operation) => ICustomParameterValuesForOperation =
35-
getCustomParameterValuesForOperation();
35+
getCustomParameterValuesByOperation();
3636

3737
for (const operation of operations) {
3838
const { associatedPhase: phase, associatedProject: project } = operation;
3939

4040
if (!operation.runner) {
4141
// This is a shell command. In the future, may consider having a property on the initial operation
4242
// to specify a runner type requested in rush-project.json
43-
const { parameterValues: customParameterValues, ignoredParameterNames } =
43+
const { parameterValues: customParameterValues, ignoredParameterValues } =
4444
getCustomParameterValues(operation);
4545

4646
const displayName: string = getDisplayName(phase, project);
@@ -65,7 +65,7 @@ export class ShellOperationRunnerPlugin implements IPhasedCommandPlugin {
6565
commandForHash,
6666
commandToRun,
6767
customParameterValues,
68-
ignoredParameterNames,
68+
ignoredParameterValues,
6969
rushConfiguration
7070
});
7171
}
@@ -85,9 +85,9 @@ export function initializeShellOperationRunner(options: {
8585
commandToRun: string | undefined;
8686
commandForHash?: string;
8787
customParameterValues: ReadonlyArray<string>;
88-
ignoredParameterNames: ReadonlyArray<string>;
88+
ignoredParameterValues: ReadonlyArray<string>;
8989
}): IOperationRunner {
90-
const { phase, project, commandToRun: rawCommandToRun, displayName, ignoredParameterNames } = options;
90+
const { phase, project, commandToRun: rawCommandToRun, displayName, ignoredParameterValues } = options;
9191

9292
if (typeof rawCommandToRun !== 'string' && phase.missingScriptBehavior === 'error') {
9393
throw new Error(
@@ -109,7 +109,7 @@ export function initializeShellOperationRunner(options: {
109109
displayName,
110110
phase,
111111
rushProject: project,
112-
ignoredParameterNames
112+
ignoredParameterValues
113113
});
114114
} else {
115115
// Empty build script indicates a no-op, so use a no-op runner
@@ -130,9 +130,9 @@ export interface ICustomParameterValuesForOperation {
130130
*/
131131
parameterValues: ReadonlyArray<string>;
132132
/**
133-
* The names of parameters that were ignored for this operation
133+
* The serialized custom parameter values that were ignored for this operation
134134
*/
135-
ignoredParameterNames: ReadonlyArray<string>;
135+
ignoredParameterValues: ReadonlyArray<string>;
136136
}
137137

138138
/**
@@ -173,9 +173,9 @@ export function getCustomParameterValuesByPhase(): (phase: IPhase) => ReadonlyAr
173173
/**
174174
* Gets custom parameter values for an operation, filtering out any parameters that should be ignored
175175
* based on the operation's settings.
176-
* @returns A function that returns the filtered custom parameter values and ignored parameter names for a given operation
176+
* @returns A function that returns the filtered custom parameter values and ignored parameter values for a given operation
177177
*/
178-
export function getCustomParameterValuesForOperation(): (
178+
export function getCustomParameterValuesByOperation(): (
179179
operation: Operation
180180
) => ICustomParameterValuesForOperation {
181181
const customParametersByPhase: Map<IPhase, Set<string>> = new Map();
@@ -195,7 +195,7 @@ export function getCustomParameterValuesForOperation(): (
195195

196196
return {
197197
parameterValues: Array.from(customParameterSet),
198-
ignoredParameterNames: []
198+
ignoredParameterValues: []
199199
};
200200
}
201201

@@ -204,23 +204,19 @@ export function getCustomParameterValuesForOperation(): (
204204
// the parameter objects to get their longName property for filtering
205205
const ignoreSet: Set<string> = new Set(parameterNamesToIgnore);
206206
const filteredParameterValues: string[] = [];
207-
const ignoredParameterNames: string[] = [];
207+
const ignoredParameterValues: string[] = [];
208208

209209
for (const tsCommandLineParameter of phase.associatedParameters) {
210210
const parameterLongName: string = tsCommandLineParameter.longName;
211211

212-
if (ignoreSet.has(parameterLongName)) {
213-
// This parameter should be ignored for this operation
214-
ignoredParameterNames.push(parameterLongName);
215-
} else {
216-
// Include this parameter in the command
217-
tsCommandLineParameter.appendToArgList(filteredParameterValues);
218-
}
212+
tsCommandLineParameter.appendToArgList(
213+
ignoreSet.has(parameterLongName) ? ignoredParameterValues : filteredParameterValues
214+
);
219215
}
220216

221217
return {
222218
parameterValues: filteredParameterValues,
223-
ignoredParameterNames
219+
ignoredParameterValues
224220
};
225221
}
226222

‎libraries/rush-lib/src/logic/operations/test/ShellOperationRunnerPlugin.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,15 @@ describe(ShellOperationRunnerPlugin.name, () => {
156156
| 'projectsInUnknownState'
157157
| 'projectConfigurations'
158158
| 'rushConfiguration'
159+
| 'customParameters'
159160
> = {
160161
phaseOriginal: buildCommand.phases,
161162
phaseSelection: buildCommand.phases,
162163
projectSelection: new Set(rushConfiguration.projects),
163164
projectsInUnknownState: new Set(rushConfiguration.projects),
164165
projectConfigurations,
165-
rushConfiguration
166+
rushConfiguration,
167+
customParameters: new Map()
166168
};
167169

168170
const hooks: PhasedCommandHooks = new PhasedCommandHooks();

‎libraries/rush-lib/src/logic/test/parameterIgnoringRepo/common/config/rush/command-line.json‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,36 @@
1313
"description": "A verbose flag",
1414
"parameterKind": "flag",
1515
"associatedCommands": ["build"]
16+
},
17+
{
18+
"longName": "--config",
19+
"description": "Config file path",
20+
"parameterKind": "string",
21+
"argumentName": "PATH",
22+
"associatedCommands": ["build"]
23+
},
24+
{
25+
"longName": "--mode",
26+
"description": "Build mode",
27+
"parameterKind": "choice",
28+
"alternatives": [
29+
{
30+
"name": "dev",
31+
"description": "Development mode"
32+
},
33+
{
34+
"name": "prod",
35+
"description": "Production mode"
36+
}
37+
],
38+
"associatedCommands": ["build"]
39+
},
40+
{
41+
"longName": "--tags",
42+
"description": "Build tags",
43+
"parameterKind": "stringList",
44+
"argumentName": "TAG",
45+
"associatedCommands": ["build"]
1646
}
1747
]
1848
}

0 commit comments

Comments
 (0)