From 6ea408c6d20700e4c1ae2aef0678c775dec311c7 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 9 Dec 2024 12:36:43 -0800 Subject: [PATCH 1/6] refactor(build-cli): Deprecate some outdated commands --- .../src/commands/generate/upcoming.ts | 7 ++++++ .../build-cli/src/commands/merge/branches.ts | 3 +++ .../commands/release/setPackageTypesField.ts | 2 ++ .../build-cli/src/library/commands/base.ts | 19 +++++---------- .../build-tools/src/common/logging.ts | 24 ++++++++++++------- 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/build-tools/packages/build-cli/src/commands/generate/upcoming.ts b/build-tools/packages/build-cli/src/commands/generate/upcoming.ts index f77d309264a7..c61111f54e3f 100644 --- a/build-tools/packages/build-cli/src/commands/generate/upcoming.ts +++ b/build-tools/packages/build-cli/src/commands/generate/upcoming.ts @@ -24,6 +24,13 @@ export default class GenerateUpcomingCommand extends BaseCommand< static readonly summary = `Generates a summary of all changesets. This is used to generate an UPCOMING.md file that provides a single place where developers can see upcoming changes.`; + // This command is deprecated and will be removed in 0.53.0. + static readonly state = "deprecated"; + static readonly deprecationOptions = { + // The version in which the deprecated command will be removed. + version: "0.53.0", + }; + // Enables the global JSON flag in oclif. static readonly enableJsonFlag = true; diff --git a/build-tools/packages/build-cli/src/commands/merge/branches.ts b/build-tools/packages/build-cli/src/commands/merge/branches.ts index 173012d336ff..fb4c45c2d866 100644 --- a/build-tools/packages/build-cli/src/commands/merge/branches.ts +++ b/build-tools/packages/build-cli/src/commands/merge/branches.ts @@ -30,6 +30,9 @@ interface CleanupBranch { export default class MergeBranch extends BaseCommand { static readonly description = "Sync branches depending on the batch size passed"; + // This command is deprecated and should no longer be used. + static readonly state = "deprecated"; + static readonly flags = { pat: Flags.string({ description: diff --git a/build-tools/packages/build-cli/src/commands/release/setPackageTypesField.ts b/build-tools/packages/build-cli/src/commands/release/setPackageTypesField.ts index 3a196e50036a..3d86fbcf89a5 100644 --- a/build-tools/packages/build-cli/src/commands/release/setPackageTypesField.ts +++ b/build-tools/packages/build-cli/src/commands/release/setPackageTypesField.ts @@ -41,6 +41,8 @@ export default class SetReleaseTagPublishingCommand extends PackageCommand< static readonly description = "Updates which .d.ts file is referenced by the `types` field in package.json. This command is used during package publishing (by CI) to select the d.ts file which corresponds to the selected API-Extractor release tag."; + // This command is deprecated and should no longer be used. + static readonly state = "deprecated"; static readonly enableJsonFlag = true; static readonly flags = { diff --git a/build-tools/packages/build-cli/src/library/commands/base.ts b/build-tools/packages/build-cli/src/library/commands/base.ts index 90ec32f9aacf..69be4df45efc 100644 --- a/build-tools/packages/build-cli/src/library/commands/base.ts +++ b/build-tools/packages/build-cli/src/library/commands/base.ts @@ -21,15 +21,6 @@ export type Flags = Interfaces.InferredFlags< >; export type Args = Interfaces.InferredArgs; -/** - * A CLI flag to parse the root directory of the Fluid repo. - */ -const rootPathFlag = Flags.custom({ - description: "Root directory of the Fluid repo (default: env _FLUID_ROOT_).", - env: "_FLUID_ROOT_", - hidden: true, -}); - /** * A base command that sets up common flags that all commands should have. Most commands should have this class in their * inheritance chain. @@ -192,17 +183,18 @@ export abstract class BaseCommand /** * Logs a warning. */ - public warning(message: string | Error | undefined): void { + public warning(message: string | Error): string | Error { if (!this.suppressLogging) { this.log(chalk.yellow(`WARNING: ${message}`)); } + return message; } /** * Logs a warning with a stack trace in debug mode. */ public warningWithDebugTrace(message: string | Error): string | Error { - return this.suppressLogging ? "" : super.warn(message); + return this.suppressLogging ? "" : this.warning(message); } // eslint-disable-next-line jsdoc/require-description @@ -210,7 +202,7 @@ export abstract class BaseCommand * @deprecated Use {@link BaseCommand.warning} or {@link BaseCommand.warningWithDebugTrace} instead. */ public warn(input: string | Error): string | Error { - return this.suppressLogging ? "" : super.warn(input); + return this.suppressLogging ? "" : this.warning(input); } /** @@ -277,10 +269,11 @@ export abstract class BaseCommand /** * Logs a verbose log statement. */ - public verbose(message: string | Error | undefined): void { + public verbose(message: string | Error): string | Error { if (this.flags.verbose === true) { const color = typeof message === "string" ? chalk.gray : chalk.red; this.log(color(`VERBOSE: ${message}`)); } + return message; } } diff --git a/build-tools/packages/build-tools/src/common/logging.ts b/build-tools/packages/build-tools/src/common/logging.ts index 576d241673b0..c60deae99155 100644 --- a/build-tools/packages/build-tools/src/common/logging.ts +++ b/build-tools/packages/build-tools/src/common/logging.ts @@ -10,12 +10,15 @@ import { commonOptions } from "../fluidBuild/commonOptions"; /** * A function that logs an Error or error message. */ -export type ErrorLoggingFunction = (msg: string | Error | undefined, ...args: any[]) => void; +export type ErrorLoggingFunction = ( + msg: string | Error, + ...args: any[] +) => string | Error | void; /** * A function that logs an error message. */ -export type LoggingFunction = (message?: string, ...args: any[]) => void; +export type LoggingFunction = (message: string, ...args: any[]) => string | void; /** * A general-purpose logger object. @@ -88,7 +91,7 @@ export const defaultLogger: Logger = { verbose, }; -function logWithTime(msg: string | Error | undefined, logFunc: ErrorLoggingFunction) { +function logWithTime(msg: string | Error, logFunc: ErrorLoggingFunction) { if (!commonOptions.logtime) { logFunc(msg); return; @@ -109,28 +112,33 @@ function logWithTime(msg: string | Error | undefined, logFunc: ErrorLoggingFunct logFunc(chalk.yellow(`[${hours}:${mins}:${secs}] `) + msg); } -function log(msg: string | undefined): void { +function log(msg: string): string { if (!commonOptions.quiet) { logWithTime(msg, console.log); } + return msg; } -function info(msg: string | Error | undefined) { +function info(msg: string | Error): string | Error { if (!commonOptions.quiet) { logWithTime(`INFO: ${msg}`, console.log); } + return msg; } -function verbose(msg: string | Error | undefined) { +function verbose(msg: string | Error): string | Error { if (!commonOptions.quiet && commonOptions.verbose) { logWithTime(`VERBOSE: ${msg}`, console.log); } + return msg; } -function warning(msg: string | Error | undefined) { +function warning(msg: string | Error): string | Error { logWithTime(`${chalk.yellow(`WARNING`)}: ${msg}`, console.log); + return msg; } -function errorLog(msg: string | Error | undefined) { +function errorLog(msg: string | Error): string | Error { logWithTime(`${chalk.red(`ERROR`)}: ${msg}`, console.error); + return msg; } From f0858466b01014e02c7017184542e51139193d41 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 9 Dec 2024 14:15:25 -0800 Subject: [PATCH 2/6] deprecations --- build-tools/packages/build-cli/.eslintrc.cjs | 5 ++++- .../src/test/library/branches.test.ts | 22 +++++++++---------- .../build-tools/src/common/gitRepo.ts | 3 +++ .../build-tools/src/common/monoRepo.ts | 3 +++ .../build-tools/src/common/npmPackage.ts | 9 ++++++++ .../build-tools/src/fluidBuild/fluidRepo.ts | 3 +++ 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/build-tools/packages/build-cli/.eslintrc.cjs b/build-tools/packages/build-cli/.eslintrc.cjs index 710cdaf3a03d..8bdbcbb148b8 100644 --- a/build-tools/packages/build-cli/.eslintrc.cjs +++ b/build-tools/packages/build-cli/.eslintrc.cjs @@ -25,6 +25,9 @@ module.exports = { // oclif uses default exports for commands "import/no-default-export": "off", + // Set to warn because we're not ready to enforce this rule yet. + "import/no-deprecated": "warn", + "import/no-internal-modules": [ "error", { @@ -52,7 +55,7 @@ module.exports = { }, ], - // Superseded by prettier and @trivago/prettier-plugin-sort-imports + // Superseded by Biome "import/order": "off", "jsdoc/multiline-blocks": [ diff --git a/build-tools/packages/build-cli/src/test/library/branches.test.ts b/build-tools/packages/build-cli/src/test/library/branches.test.ts index ee6c4ceeacfb..21edfc7b0d0c 100644 --- a/build-tools/packages/build-cli/src/test/library/branches.test.ts +++ b/build-tools/packages/build-cli/src/test/library/branches.test.ts @@ -6,8 +6,6 @@ import { assert } from "chai"; import { describe, it } from "mocha"; -import { MonoRepoKind } from "../../library/index.js"; - import { generateBumpDepsBranchName, generateBumpVersionBranchName, @@ -60,19 +58,19 @@ describe("generateBumpVersionBranchName", () => { describe("generateBumpDepsBranchName", () => { describe("semver versions", () => { it("patch", () => { - const actual = generateBumpDepsBranchName(MonoRepoKind.Azure, "patch"); + const actual = generateBumpDepsBranchName("azure", "patch"); const expected = "bump_deps_azure_patch"; assert.equal(actual, expected); }); it("minor", () => { - const actual = generateBumpDepsBranchName(MonoRepoKind.Azure, "minor"); + const actual = generateBumpDepsBranchName("azure", "minor"); const expected = "bump_deps_azure_minor"; assert.equal(actual, expected); }); it("major", () => { - const actual = generateBumpDepsBranchName(MonoRepoKind.Azure, "major"); + const actual = generateBumpDepsBranchName("azure", "major"); const expected = "bump_deps_azure_major"; assert.equal(actual, expected); }); @@ -101,43 +99,43 @@ describe("generateBumpDepsBranchName", () => { describe("generateReleaseBranchName", () => { it("semver", () => { - const actual = generateReleaseBranchName(MonoRepoKind.Azure, "1.2.3"); + const actual = generateReleaseBranchName("azure", "1.2.3"); const expected = "release/azure/1.2"; assert.equal(actual, expected); }); it("virtualPatch version scheme", () => { - const actual = generateReleaseBranchName(MonoRepoKind.BuildTools, "0.4.2000"); + const actual = generateReleaseBranchName("build-tools", "0.4.2000"); const expected = "release/build-tools/0.4.2000"; assert.equal(actual, expected); }); it("virtualPatch patch", () => { - const actual = generateReleaseBranchName(MonoRepoKind.BuildTools, "0.4.2002"); + const actual = generateReleaseBranchName("build-tools", "0.4.2002"); const expected = "release/build-tools/0.4.2000"; assert.equal(actual, expected); }); it("client using semver < 2.0.0", () => { - const actual = generateReleaseBranchName(MonoRepoKind.Client, "1.2.3"); + const actual = generateReleaseBranchName("client", "1.2.3"); const expected = "release/client/1.2"; assert.equal(actual, expected); }); it("client using semver >= 2.0.0", () => { - const actual = generateReleaseBranchName(MonoRepoKind.Client, "2.0.0"); + const actual = generateReleaseBranchName("client", "2.0.0"); const expected = "release/client/2.0"; assert.equal(actual, expected); }); it("Fluid internal version scheme", () => { - const actual = generateReleaseBranchName(MonoRepoKind.Client, "2.0.0-internal.1.0.0"); + const actual = generateReleaseBranchName("client", "2.0.0-internal.1.0.0"); const expected = "release/v2int/1.0"; assert.equal(actual, expected); }); it("Fluid RC version scheme", () => { - const actual = generateReleaseBranchName(MonoRepoKind.Client, "2.0.0-rc.1.0.0"); + const actual = generateReleaseBranchName("client", "2.0.0-rc.1.0.0"); const expected = "release/client/2.0.0-rc.1.0"; assert.equal(actual, expected); }); diff --git a/build-tools/packages/build-tools/src/common/gitRepo.ts b/build-tools/packages/build-tools/src/common/gitRepo.ts index 69b046522940..2d7345f472e5 100644 --- a/build-tools/packages/build-tools/src/common/gitRepo.ts +++ b/build-tools/packages/build-tools/src/common/gitRepo.ts @@ -9,6 +9,9 @@ import { exec, execNoError } from "./utils"; const traceGitRepo = registerDebug("fluid-build:gitRepo"); +/** + * @deprecated Should not be used outside the build-tools package. + */ export class GitRepo { constructor(public readonly resolvedRoot: string) {} diff --git a/build-tools/packages/build-tools/src/common/monoRepo.ts b/build-tools/packages/build-tools/src/common/monoRepo.ts index f8b7f3e4ad8f..ff66921a9247 100644 --- a/build-tools/packages/build-tools/src/common/monoRepo.ts +++ b/build-tools/packages/build-tools/src/common/monoRepo.ts @@ -38,6 +38,9 @@ export type PackageManager = "npm" | "pnpm" | "yarn"; * - If package.json contains a workspaces field, then packages will be loaded based on the globs in that field. * * - If the version was not defined in lerna.json, then the version value in package.json will be used. + * + * + * @deprecated Should not be used outside the build-tools package. */ export class MonoRepo { public readonly packages: Package[] = []; diff --git a/build-tools/packages/build-tools/src/common/npmPackage.ts b/build-tools/packages/build-tools/src/common/npmPackage.ts index 1155718cfeae..796b75fc89ed 100644 --- a/build-tools/packages/build-tools/src/common/npmPackage.ts +++ b/build-tools/packages/build-tools/src/common/npmPackage.ts @@ -72,6 +72,9 @@ interface PackageDependency { depClass: "prod" | "dev" | "peer"; } +/** + * @deprecated Should not be used outside the build-tools package. + */ export class Package { private static packageCount: number = 0; private static readonly chalkColor = [ @@ -505,6 +508,8 @@ export class Packages { * The package.json is always sorted using sort-package-json. * * @internal + * + * @deprecated Should not be used outside the build-tools package. */ export function updatePackageJsonFile( packagePath: string, @@ -526,6 +531,8 @@ export function updatePackageJsonFile( * indentation. * * @internal + * + * @deprecated Should not be used outside the build-tools package. */ export function readPackageJsonAndIndent( pathToJson: string, @@ -556,6 +563,8 @@ function writePackageJson(packagePath: string, pkgJson: PackageJson, indent: str * The package.json is always sorted using sort-package-json. * * @internal + * + * @deprecated Should not be used outside the build-tools package. */ export async function updatePackageJsonFileAsync( packagePath: string, diff --git a/build-tools/packages/build-tools/src/fluidBuild/fluidRepo.ts b/build-tools/packages/build-tools/src/fluidBuild/fluidRepo.ts index ca2a18158724..d143d3750337 100644 --- a/build-tools/packages/build-tools/src/fluidBuild/fluidRepo.ts +++ b/build-tools/packages/build-tools/src/fluidBuild/fluidRepo.ts @@ -14,6 +14,9 @@ import { type IFluidBuildDirs, } from "./fluidBuildConfig"; +/** + * @deprecated Should not be used outside the build-tools package. + */ export class FluidRepo { private readonly _releaseGroups = new Map(); From 179330438a3ea23723c8366a454179ec2e998daa Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 9 Dec 2024 15:10:22 -0800 Subject: [PATCH 3/6] remove Timer export --- build-tools/packages/build-cli/src/commands/check/layers.ts | 6 ++---- build-tools/packages/build-tools/src/index.ts | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/build-tools/packages/build-cli/src/commands/check/layers.ts b/build-tools/packages/build-cli/src/commands/check/layers.ts index de75675b948a..580a100706c2 100644 --- a/build-tools/packages/build-cli/src/commands/check/layers.ts +++ b/build-tools/packages/build-cli/src/commands/check/layers.ts @@ -6,7 +6,6 @@ import { writeFile } from "node:fs/promises"; import path from "node:path"; -import { Timer } from "@fluidframework/build-tools"; import { Flags } from "@oclif/core"; import { BaseCommand, LayerGraph } from "../../library/index.js"; @@ -40,12 +39,11 @@ export class CheckLayers extends BaseCommand { async run(): Promise { const { flags } = this; - const timer = new Timer(flags.timer); const context = await this.getContext(); const { packages, resolvedRoot } = context.repo; - timer.time("Package scan completed"); + this.verbose("Package scan completed"); const layerGraph = LayerGraph.load(resolvedRoot, packages.packages, flags.info); @@ -64,7 +62,7 @@ export class CheckLayers extends BaseCommand { } const success: boolean = layerGraph.verify(); - timer.time("Layer check completed"); + this.verbose("Layer check completed"); if (!success) { this.error("Layer check not succesful"); diff --git a/build-tools/packages/build-tools/src/index.ts b/build-tools/packages/build-tools/src/index.ts index 7af3d2e815c2..0418ec1efd59 100644 --- a/build-tools/packages/build-tools/src/index.ts +++ b/build-tools/packages/build-tools/src/index.ts @@ -15,7 +15,6 @@ export { updatePackageJsonFile, updatePackageJsonFileAsync, } from "./common/npmPackage"; -export { Timer } from "./common/timer"; // For repo policy check export { From 95171680ee04400bcfda72d2b40321d2f250d4b5 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 9 Dec 2024 16:41:20 -0800 Subject: [PATCH 4/6] logtime flag --- build-tools/packages/build-cli/docs/check.md | 3 +-- build-tools/packages/build-cli/src/commands/check/layers.ts | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/build-tools/packages/build-cli/docs/check.md b/build-tools/packages/build-cli/docs/check.md index 29e21e1f4ee1..9c7981850e52 100644 --- a/build-tools/packages/build-cli/docs/check.md +++ b/build-tools/packages/build-cli/docs/check.md @@ -131,12 +131,11 @@ Checks that the dependencies between Fluid Framework packages are properly layer ``` USAGE - $ flub check layers --info [-v | --quiet] [--md ] [--dot ] [--logtime] + $ flub check layers --info [-v | --quiet] [--md ] [--dot ] FLAGS --dot= Generate *.dot for GraphViz --info= (required) Path to the layer graph json file - --logtime Display the current time on every status message for logging --md= Generate PACKAGES.md file at this path relative to repo root LOGGING FLAGS diff --git a/build-tools/packages/build-cli/src/commands/check/layers.ts b/build-tools/packages/build-cli/src/commands/check/layers.ts index 580a100706c2..13f0e0dca717 100644 --- a/build-tools/packages/build-cli/src/commands/check/layers.ts +++ b/build-tools/packages/build-cli/src/commands/check/layers.ts @@ -30,10 +30,6 @@ export class CheckLayers extends BaseCommand { required: true, exists: true, }), - logtime: Flags.boolean({ - description: "Display the current time on every status message for logging", - required: false, - }), ...BaseCommand.flags, } as const; From 66e25d644033110b9fa2830dd7023a739273796a Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Tue, 10 Dec 2024 11:52:20 -0800 Subject: [PATCH 5/6] Update build-tools/packages/build-tools/src/common/monoRepo.ts --- build-tools/packages/build-tools/src/common/monoRepo.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/build-tools/packages/build-tools/src/common/monoRepo.ts b/build-tools/packages/build-tools/src/common/monoRepo.ts index ff66921a9247..9129450beef7 100644 --- a/build-tools/packages/build-tools/src/common/monoRepo.ts +++ b/build-tools/packages/build-tools/src/common/monoRepo.ts @@ -39,7 +39,6 @@ export type PackageManager = "npm" | "pnpm" | "yarn"; * * - If the version was not defined in lerna.json, then the version value in package.json will be used. * - * * @deprecated Should not be used outside the build-tools package. */ export class MonoRepo { From 68c36ecc015ed83928db3843784dbb919ce9d6e1 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Tue, 10 Dec 2024 12:11:48 -0800 Subject: [PATCH 6/6] lint config --- build-tools/packages/build-cli/.eslintrc.cjs | 10 +++++++++- .../build-cli/src/commands/generate/upcoming.ts | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/build-tools/packages/build-cli/.eslintrc.cjs b/build-tools/packages/build-cli/.eslintrc.cjs index 8bdbcbb148b8..67298a2d42ca 100644 --- a/build-tools/packages/build-cli/.eslintrc.cjs +++ b/build-tools/packages/build-cli/.eslintrc.cjs @@ -25,7 +25,7 @@ module.exports = { // oclif uses default exports for commands "import/no-default-export": "off", - // Set to warn because we're not ready to enforce this rule yet. + // Set to warn because we're not ready to enforce this rule for much of build-cli yet. It is enabled for new code. "import/no-deprecated": "warn", "import/no-internal-modules": [ @@ -105,5 +105,13 @@ module.exports = { "import/no-internal-modules": "off", }, }, + { + // Rules only for files that are built on the build-infrastructure APIs. + files: ["src/**/vnext/**"], + rules: { + // Set to error since code using build-infrastructure APIs should not need to use any deprecated APIs. + "import/no-deprecated": "error", + }, + }, ], }; diff --git a/build-tools/packages/build-cli/src/commands/generate/upcoming.ts b/build-tools/packages/build-cli/src/commands/generate/upcoming.ts index c61111f54e3f..ddaa14ec0f86 100644 --- a/build-tools/packages/build-cli/src/commands/generate/upcoming.ts +++ b/build-tools/packages/build-cli/src/commands/generate/upcoming.ts @@ -29,6 +29,8 @@ export default class GenerateUpcomingCommand extends BaseCommand< static readonly deprecationOptions = { // The version in which the deprecated command will be removed. version: "0.53.0", + // The replacement command. + to: "generate releaseNotes", }; // Enables the global JSON flag in oclif.