Skip to content
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

feat: Make flub release request bump type lazily #23096

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions build-tools/packages/build-cli/src/commands/release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
StateHandler,
} from "../handlers/index.js";
import { PromptWriter } from "../instructionalPromptWriter.js";
import { Lazy } from "../lazy.js";
// eslint-disable-next-line import/no-deprecated
import { MonoRepoKind, getDefaultBumpTypeForBranch } from "../library/index.js";
import { FluidReleaseMachine } from "../machines/index.js";
Expand Down Expand Up @@ -97,14 +98,18 @@ export default class ReleaseCommand extends StateMachineCommand<typeof ReleaseCo
const releaseVersion = packageOrReleaseGroup.version;

const currentBranch = await context.gitRepo.getCurrentBranchName();
const bumpType = await getBumpType(flags.bumpType, currentBranch, releaseVersion);
const bumpType = new Lazy(async () => {
const bumpTypeInner = await getBumpType(flags.bumpType, currentBranch, releaseVersion);

// eslint-disable-next-line no-warning-comments
// TODO: can be removed once server team owns server releases
// eslint-disable-next-line import/no-deprecated
if (flags.releaseGroup === MonoRepoKind.Server && bumpType === "minor") {
this.error(`Server release are always a ${chalk.bold("MAJOR")} release`);
}
// eslint-disable-next-line no-warning-comments
// TODO: can be removed once server team owns server releases
// eslint-disable-next-line import/no-deprecated
if (flags.releaseGroup === MonoRepoKind.Server && bumpTypeInner === "minor") {
this.error(`Server release are always a ${chalk.bold("MAJOR")} release`);
}

return bumpTypeInner;
});

// oclif doesn't support nullable boolean flags, so this works around that limitation by checking the args
// passed into the command. If neither are passed, then the default is determined by the branch config.
Expand Down
33 changes: 2 additions & 31 deletions build-tools/packages/build-cli/src/handlers/askFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@
* Licensed under the MIT License.
*/

import { type VersionBumpType, bumpVersionScheme } from "@fluid-tools/version-tools";
import { rawlist } from "@inquirer/prompts";
import { Machine } from "jssm";

import { getDefaultBumpTypeForBranch } from "../library/index.js";
import { CommandLogger } from "../logging.js";
import { MachineState } from "../machines/index.js";
import { FluidReleaseStateHandlerData } from "./fluidReleaseStateHandler.js";
Expand All @@ -32,34 +29,8 @@ export const askForReleaseType: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { bumpType: inputBumpType, context, releaseVersion } = data;

const currentBranch = await context.gitRepo.getCurrentBranchName();
const currentVersion = releaseVersion;
const bumpedMajor = bumpVersionScheme(currentVersion, "major");
const bumpedMinor = bumpVersionScheme(currentVersion, "minor");
const bumpedPatch = bumpVersionScheme(currentVersion, "patch");

// If an bumpType was set in the handler data, use it. Otherwise set it as the default for the branch. If there's
// no default for the branch, ask the user.
let bumpType = inputBumpType ?? getDefaultBumpTypeForBranch(currentBranch);
if (inputBumpType === undefined) {
const selectedBumpType: VersionBumpType = await rawlist({
choices: [
{ value: "major", name: `major (${currentVersion} => ${bumpedMajor.version})` },
{ value: "minor", name: `minor (${currentVersion} => ${bumpedMinor.version})` },
{ value: "patch", name: `patch (${currentVersion} => ${bumpedPatch.version})` },
],
message: `The current branch is '${currentBranch}'. The default bump type for that branch is '${bumpType}', but you can change it now if needed.`,
});
bumpType = selectedBumpType;
// eslint-disable-next-line require-atomic-updates
data.bumpType = selectedBumpType;
}

if (bumpType === undefined) {
throw new Error(`bumpType is undefined.`);
}
Comment on lines -35 to -62
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this logical basically does nothing because inputBumpType can't be undefined based on its type. This also seems redundant with the logic which computes the inputBumpType. Logic has been simplified to use the already provided bump type (which will lazily prompt if needed)

const { bumpType: bumpTypeLazy } = data;
const bumpType = await bumpTypeLazy.value;

// This state is unique; it uses major/minor/patch as the actions
const result = machine.action(bumpType);
Expand Down
18 changes: 12 additions & 6 deletions build-tools/packages/build-cli/src/handlers/checkFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ export const checkBranchName: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { context, bumpType, shouldCheckBranch } = data;
const { context, bumpType: bumpTypeLazy, shouldCheckBranch } = data;
const bumpType = await bumpTypeLazy.value;

if (shouldCheckBranch === true) {
switch (bumpType) {
Expand Down Expand Up @@ -276,7 +277,7 @@ export const checkMainNextIntegrated: StateHandlerFunction = async (

const { bumpType, context, shouldCheckMainNextIntegrated } = data;

if (bumpType === "major") {
if ((await bumpType.value) === "major") {
if (shouldCheckMainNextIntegrated === true) {
const [main, next] = await Promise.all([
context.gitRepo.getShaForBranch("main"),
Expand Down Expand Up @@ -508,7 +509,8 @@ export const checkReleaseNotes: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { bumpType, releaseGroup, releaseVersion } = data;
const { bumpType: bumpTypeLazy, releaseGroup, releaseVersion } = data;
const bumpType = await bumpTypeLazy.value;

if (
// Only some release groups use changeset-based change-tracking.
Expand Down Expand Up @@ -563,7 +565,8 @@ export const checkChangelogs: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { releaseGroup, bumpType } = data;
const { releaseGroup, bumpType: bumpTypeLazy } = data;
const bumpType = await bumpTypeLazy.value;

if (
// Only some release groups use changeset-based change-tracking.
Expand Down Expand Up @@ -641,10 +644,12 @@ export const checkReleaseGroupIsBumped: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { context, releaseGroup, releaseVersion, bumpType } = data;
const { context, releaseGroup, releaseVersion, bumpType: bumpTypeLazy } = data;

context.repo.reload();
const repoVersion = context.getVersion(releaseGroup);

const bumpType = await bumpTypeLazy.value;
const targetVersion = bumpVersionScheme(releaseVersion, bumpType).version;

if (repoVersion !== targetVersion) {
Expand Down Expand Up @@ -707,13 +712,14 @@ export const checkShouldCommit: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { bumpType, context, shouldCommit, releaseGroup, releaseVersion } = data;
const { bumpType: bumpTypeLazy, context, shouldCommit, releaseGroup, releaseVersion } = data;

if (shouldCommit !== true) {
BaseStateHandler.signalFailure(machine, state);
return true;
}

const bumpType = await bumpTypeLazy.value;
const branchName = generateBumpVersionBranchName(releaseGroup, bumpType, releaseVersion);
const commitMsg = generateBumpVersionCommitMessage(releaseGroup, bumpType, releaseVersion);

Expand Down
11 changes: 10 additions & 1 deletion build-tools/packages/build-cli/src/handlers/doFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,13 @@ export const doReleaseGroupBump: StateHandlerFunction = async (
): Promise<boolean> => {
if (testMode) return true;

const { bumpType, context, releaseGroup, releaseVersion, shouldInstall } = data;
const {
bumpType: bumpTypeLazy,
context,
releaseGroup,
releaseVersion,
shouldInstall,
} = data;

const rgRepo = isReleaseGroup(releaseGroup)
? // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Expand All @@ -165,6 +171,9 @@ export const doReleaseGroupBump: StateHandlerFunction = async (
context.fullPackageMap.get(releaseGroup)!;

const scheme = detectVersionScheme(releaseVersion);

const bumpType = await bumpTypeLazy.value;

const newVersion = bumpVersionScheme(releaseVersion, bumpType, scheme);
const packages = rgRepo instanceof MonoRepo ? rgRepo.packages : [rgRepo];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Context } from "../library/index.js";
import { ReleaseVersion, VersionBumpType, VersionScheme } from "@fluid-tools/version-tools";

import { InstructionalPromptWriter } from "../instructionalPromptWriter.js";
import type { Lazy } from "../lazy.js";
import { CommandLogger } from "../logging.js";
import { MachineState } from "../machines/index.js";
import { ReleaseGroup, ReleasePackage } from "../releaseGroups.js";
Expand Down Expand Up @@ -79,7 +80,7 @@ export interface FluidReleaseStateHandlerData {
/**
* The bump type used for this release.
*/
bumpType: VersionBumpType;
bumpType: Lazy<Promise<VersionBumpType>>;

/**
* An {@link InstructionalPromptWriter} that the command can use to display instructional prompts.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { type InstructionalPrompt, mapADOLinks } from "../instructionalPromptWri
import {
difference,
generateReleaseBranchName,
getDefaultBumpTypeForBranch,
getPreReleaseDependencies,
} from "../library/index.js";
import { CommandLogger } from "../logging.js";
Expand Down Expand Up @@ -486,16 +485,13 @@ export const promptToGenerateReleaseNotes: StateHandlerFunction = async (

const {
command,
context,
promptWriter,
releaseGroup,
releaseVersion,
bumpType: inputBumpType,
} = data;
const currentBranch = await context.gitRepo.getCurrentBranchName();

// If an bumpType was set in the handler data, use it. Otherwise set it as the default for the branch.
const bumpType = inputBumpType ?? getDefaultBumpTypeForBranch(currentBranch);
const bumpType = await inputBumpType.value;

const prompt: InstructionalPrompt = {
title: "NEED TO GENERATE RELEASE NOTES",
Expand Down
36 changes: 36 additions & 0 deletions build-tools/packages/build-cli/src/lazy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

/**
* Helper class for lazy initialized values.
* Ensures the value is only generated once.
*/
export class Lazy<T> {
/**
* Sentinel value that valueGenerator could not return (unlike undefined) for the unset case.
*/
private static readonly unset: unique symbol = Symbol("unset");

/**
* The value, if computed, otherwise `unset`.
*/
private lazyValue: T | typeof Lazy.unset = Lazy.unset;

/**
* Instantiates an instance of Lazy<T>.
* @param valueGenerator - The function that will generate the value when value is accessed the first time.
*/
public constructor(private readonly valueGenerator: () => T) {}

/**
* Get the value. If this is the first call the value will be generated.
*/
public get value(): T {
if (this.lazyValue === Lazy.unset) {
this.lazyValue = this.valueGenerator();
}
return this.lazyValue;
}
}
Loading