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

refactor(build-tools): Deprecate some outdated commands and APIs #23280

Merged
merged 7 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
13 changes: 12 additions & 1 deletion build-tools/packages/build-cli/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 for much of build-cli yet. It is enabled for new code.
"import/no-deprecated": "warn",

"import/no-internal-modules": [
"error",
{
Expand Down Expand Up @@ -52,7 +55,7 @@ module.exports = {
},
],

// Superseded by prettier and @trivago/prettier-plugin-sort-imports
// Superseded by Biome
"import/order": "off",

"jsdoc/multiline-blocks": [
Expand Down Expand Up @@ -102,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",
},
},
],
};
3 changes: 1 addition & 2 deletions build-tools/packages/build-cli/docs/check.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,11 @@ Checks that the dependencies between Fluid Framework packages are properly layer

```
USAGE
$ flub check layers --info <value> [-v | --quiet] [--md <value>] [--dot <value>] [--logtime]
$ flub check layers --info <value> [-v | --quiet] [--md <value>] [--dot <value>]

FLAGS
--dot=<value> Generate *.dot for GraphViz
--info=<value> (required) Path to the layer graph json file
--logtime Display the current time on every status message for logging
--md=<value> Generate PACKAGES.md file at this path relative to repo root

LOGGING FLAGS
Expand Down
10 changes: 2 additions & 8 deletions build-tools/packages/build-cli/src/commands/check/layers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -31,21 +30,16 @@ export class CheckLayers extends BaseCommand<typeof CheckLayers> {
required: true,
exists: true,
}),
logtime: Flags.boolean({
description: "Display the current time on every status message for logging",
required: false,
}),
...BaseCommand.flags,
} as const;

async run(): Promise<void> {
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);

Expand All @@ -64,7 +58,7 @@ export class CheckLayers extends BaseCommand<typeof CheckLayers> {
}

const success: boolean = layerGraph.verify();
timer.time("Layer check completed");
this.verbose("Layer check completed");

if (!success) {
this.error("Layer check not succesful");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ 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",
// The replacement command.
to: "generate releaseNotes",
};

// Enables the global JSON flag in oclif.
static readonly enableJsonFlag = true;

Expand Down
3 changes: 3 additions & 0 deletions build-tools/packages/build-cli/src/commands/merge/branches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ interface CleanupBranch {
export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
19 changes: 6 additions & 13 deletions build-tools/packages/build-cli/src/library/commands/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@ export type Flags<T extends typeof Command> = Interfaces.InferredFlags<
>;
export type Args<T extends typeof Command> = Interfaces.InferredArgs<T["args"]>;

/**
* A CLI flag to parse the root directory of the Fluid repo.
*/
const rootPathFlag = Flags.custom({
Copy link
Member Author

Choose a reason for hiding this comment

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

Was unused.

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.
Expand Down Expand Up @@ -190,25 +181,26 @@ export abstract class BaseCommand<T extends typeof Command>
/**
* 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
/**
* @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);
}

/**
Expand Down Expand Up @@ -275,10 +267,11 @@ export abstract class BaseCommand<T extends typeof Command>
/**
* 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;
}
}
22 changes: 10 additions & 12 deletions build-tools/packages/build-cli/src/test/library/branches.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
import { assert } from "chai";
import { describe, it } from "mocha";

import { MonoRepoKind } from "../../library/index.js";

import {
generateBumpDepsBranchName,
generateBumpVersionBranchName,
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
});
Expand Down
3 changes: 3 additions & 0 deletions build-tools/packages/build-tools/src/common/gitRepo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
24 changes: 16 additions & 8 deletions build-tools/packages/build-tools/src/common/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
2 changes: 2 additions & 0 deletions build-tools/packages/build-tools/src/common/monoRepo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ 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.
*
tylerbutler marked this conversation as resolved.
Show resolved Hide resolved
* @deprecated Should not be used outside the build-tools package.
*/
export class MonoRepo {
public readonly packages: Package[] = [];
Expand Down
9 changes: 9 additions & 0 deletions build-tools/packages/build-tools/src/common/npmPackage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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,
Expand All @@ -526,6 +531,8 @@ export function updatePackageJsonFile(
* indentation.
*
* @internal
*
* @deprecated Should not be used outside the build-tools package.
*/
export function readPackageJsonAndIndent(
pathToJson: string,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions build-tools/packages/build-tools/src/fluidBuild/fluidRepo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, MonoRepo>();

Expand Down
Loading
Loading