-
Notifications
You must be signed in to change notification settings - Fork 558
(compat) Updated build-tools to check that layer generation is updated during release #25873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -430,7 +430,7 @@ export const checkPolicy: StateHandlerFunction = async ( | |
| }; | ||
|
|
||
| /** | ||
| * Checks that a release branch exists. | ||
| * Checks whether assert tagging has been done. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! |
||
| * | ||
| * @param state - The current state machine state. | ||
| * @param machine - The state machine. | ||
|
|
@@ -903,3 +903,54 @@ export const checkValidReleaseGroup: StateHandlerFunction = async ( | |
|
|
||
| return true; | ||
| }; | ||
|
|
||
| /** | ||
| * Checks whether layer compat generation needs to be updated. | ||
| * | ||
| * @param state - The current state machine state. | ||
| * @param machine - The state machine. | ||
| * @param testMode - Set to true to run function in test mode. | ||
| * @param log - A logger that the function can use for logging. | ||
| * @param data - An object with handler-specific contextual data. | ||
| * @returns True if the state was handled; false otherwise. | ||
| */ | ||
| export const checkLayerCompatGeneration: StateHandlerFunction = async ( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The terminology still really confuses me here. We have a "layer check" tool. Are these the same layers? |
||
| state: MachineState, | ||
| machine: Machine<unknown>, | ||
| testMode: boolean, | ||
| log: CommandLogger, | ||
| data: FluidReleaseStateHandlerData, | ||
| ): Promise<boolean> => { | ||
| if (testMode) return true; | ||
|
|
||
| const { context, bumpType } = data; | ||
| const gitRepo = await context.getGitRepository(); | ||
|
|
||
| if (bumpType === "patch") { | ||
| log.verbose(`Skipping layer compat generation check for patch release.`); | ||
| BaseStateHandler.signalSuccess(machine, state); | ||
| return true; | ||
| } | ||
|
|
||
| // layerGeneration:gen should be run from the root. It will only update packages that have the layerGeneration:gen | ||
| // script defined in their package.json. | ||
| const result = await execa.command(`pnpm run -r layerGeneration:gen`, { | ||
| cwd: context.root, | ||
| }); | ||
| log.verbose(result.stdout); | ||
|
Comment on lines
+937
to
+940
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason we can't call the code that this command invokes? Exec-ing is the least preferred method of integrating if we can just call a function. |
||
|
|
||
| // check for policy check violation | ||
| const afterPolicyCheckStatus = await gitRepo.gitClient.status(); | ||
| const isClean = afterPolicyCheckStatus.isClean(); | ||
| if (!isClean) { | ||
| log.logHr(); | ||
| log.errorLog( | ||
| `Layer generation needs to be updated. Please create a PR for the changes and merge before retrying.\n${afterPolicyCheckStatus.files.map((fileStatus) => `${fileStatus.index} ${fileStatus.path}`).join("\n")}`, | ||
| ); | ||
| BaseStateHandler.signalFailure(machine, state); | ||
| return false; | ||
| } | ||
|
|
||
| BaseStateHandler.signalSuccess(machine, state); | ||
| return true; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What this done intentionally? I don't recall seeing a corresponding change to the command that generates this?