Move common code + class definition updates

.github/shared/package.json

@@ -1,3 +1,3 @@
 {
   "name": "@azure-tools/specs-shared",
   "private": "true",
@@ -20,7 +20,8 @@
     "./swagger": "./src/swagger.js",
     "./tag": "./src/tag.js",
     "./simple-git": "./src/simple-git.js",
-    "./test/examples": "./test/examples.js"
+    "./test/examples": "./test/examples.js",
+    "./branch": "./src/branch.js"
   },
   "bin": {
     "spec-model": "./cmd/spec-model.js"

.github/shared/src/branch.js

@@ -0,0 +1,9 @@
+/**
+ * Checks an input branch name and determines if it should be considered a release branch.
+ * @param {string} branchName
+ * @returns {boolean}
+ */
+export function isReleaseBranch(branchName) {
+  const branchRegex = [/main/, /RPSaaSMaster/, /release*/, /ARMCoreRPDev/];
+  return branchRegex.some((b) => b.test(branchName));
+}

.github/shared/test/branch.test.js

@@ -0,0 +1,36 @@
+import { describe, test, expect } from "vitest";
+import { isReleaseBranch } from "../src/branch.js";
+
+describe("isReleaseBranch", () => {
+  test("returns true for main branch", () => {
+    expect(isReleaseBranch("main")).toBe(true);
+  });
+
+  test("returns true for RPSaaSMaster branch", () => {
+    expect(isReleaseBranch("RPSaaSMaster")).toBe(true);
+  });
+
+  test("returns true for release branches", () => {
+    expect(isReleaseBranch("release")).toBe(true);
+    expect(isReleaseBranch("release-v1.0")).toBe(true);
+    expect(isReleaseBranch("release/feature")).toBe(true);
+    expect(isReleaseBranch("releasebranch")).toBe(true);
+  });
+
+  test("returns true for ARMCoreRPDev branch", () => {
+    expect(isReleaseBranch("ARMCoreRPDev")).toBe(true);
+  });
+
+  test("returns false for feature branches", () => {
+    expect(isReleaseBranch("feature/new-feature")).toBe(false);
+    expect(isReleaseBranch("bugfix/fix-issue")).toBe(false);
+    expect(isReleaseBranch("develop")).toBe(false);
+    expect(isReleaseBranch("code-cleanup-classes")).toBe(false);
+  });
+
+  test("returns false for empty or invalid branch names", () => {
+    expect(isReleaseBranch("")).toBe(false);
+    expect(isReleaseBranch(" ")).toBe(false);
+    expect(isReleaseBranch("test-branch")).toBe(false);
+  });
+});

.github/workflows/src/summarize-checks/labelling.js

@@ -1,3 +1,3 @@
 /*
     This file covers two areas of enforcement:
     1. It calculates what set of label rules has been violated by the current PR, for the purposes of updating next steps to merge.
@@ -12,6 +12,8 @@
   wrapInArmReviewMessage,
 } from "./tsgs.js";
 
+import { isReleaseBranch } from "../../../shared/src/branch.js";
+
 // #region typedefs
 /**
  * The LabelContext is used by the updateLabels() to determine which labels to add or remove to the PR.
@@ -190,31 +192,36 @@
  *
  * See also: https://aka.ms/SpecPRReviewARMInvariants
  */
-// todo: inject `core` for access to logging
 export class Label {
+  /**
+   * The name of the label.
+   * @type {string}
+   */
+  name;
+
+  /**
+   * Is the label currently present on the pull request?
+   * @type {boolean}
+   */
+  present;
+
+  /**
+   * Should this label be present on the pull request?
+   * Must be defined before applyStateChange is called.
+   * Not set at the construction time to facilitate determining desired presence
+   * of multiple labels in single code block, without intermixing it with
+   * label construction logic.
+   * @type {boolean | undefined}
+   */
+  shouldBePresent;
+
   /**
    * @param {string} name
    * @param {Set<string>} [presentLabels]
    */
   constructor(name, presentLabels) {
-    /** @type {string} */
     this.name = name;
-
-    /**
-     * Is the label currently present on the pull request?
-     * This is determined at the time of construction of this object.
-     * @type {boolean | undefined}
-     */
-    this.present = presentLabels?.has(this.name) ?? undefined;
-
-    /**
-     * Should this label be present on the pull request?
-     * Must be defined before applyStateChange is called.
-     * Not set at the construction time to facilitate determining desired presence
-     * of multiple labels in single code block, without intermixing it with
-     * label construction logic.
-     * @type {boolean | undefined}
-     */
+    this.present = presentLabels?.has(this.name) ?? false;
     this.shouldBePresent = undefined;
   }
 
@@ -273,25 +280,6 @@
       );
     }
   }
-
-  /**
-   * @param {string} label
-   * @returns {boolean}
-   */
-  isEqualToOrPrefixOf(label) {
-    return this.name.endsWith("*") ? label.startsWith(this.name.slice(0, -1)) : this.name === label;
-  }
-
-  /**
-   * @returns {string}
-   */
-  logString() {
-    return (
-      `Label: name: ${this.name}, ` +
-      `present: ${this.present}, ` +
-      `shouldBePresent: ${this.shouldBePresent}. `
-    );
-  }
 }
 
 // #endregion typedefs
@@ -567,15 +555,6 @@
   return { armReviewLabelShouldBePresent: armReviewLabel.shouldBePresent };
 }
 
-/**
- * @param {string} branchName
- * @returns {boolean}
- */
-function isReleaseBranch(branchName) {
-  const branchRegex = [/main/, /RPSaaSMaster/, /release*/, /ARMCoreRPDev/];
-  return branchRegex.some((b) => b.test(branchName));
-}
-
 /**
 CODESYNC:
 - requiredLabelsRules.ts / requiredLabelsRules

eng/tools/summarize-impact/src/PRContext.ts

@@ -1,3 +1,3 @@
 import { join, dirname } from "path";
 import * as fs from "fs";
 
@@ -36,22 +36,75 @@
   isDraft: boolean;
 };
 
+/**
+ * Represents the context of a Pull Request (PR) in a repository.
+ * It contains information about the source and target directories, branches, SHA, and other relevant details while
+ * providing methods to abstract the differences between the source and target states.
+ */
 export class PRContext {
-  // we are starting with checking out before and after to different directories
-  // sourceDirectory corresponds to "after". EG the PR context is the "after" state of the PR.
-  // The "before" state is the git root directory without the changes aka targetDirectory
+  /**
+   * Path to the source directory representing the code actually being changed.
+   *
+   * This directory contains all the files as they exist on the PR branch and is
+   * compared against `targetDirectory` to compute additions, deletions, and modifications.
+   */
   sourceDirectory: string;
+  /**
+   * Path to the source directory representing the code before the changes in the PR.
+   *
+   * This directory contains all the files as they exist on the target branch and is
+   * compared against `sourceDirectory` to compute additions, deletions, and modifications.
+   */
   targetDirectory: string;
+  /**
+   * The spec model for the source directory.
+   *
+   * This is used to analyze the TypeSpec files in the source directory.
+   */
   sourceSpecModel?: SpecModel;
+  /**
+   * The spec model for the target directory.
+   *
+   * This is used to analyze the TypeSpec files in the target directory.
+   */
   targetSpecModel?: SpecModel;
+  /**
+   * The information generated about the PR from getChangedFileStatuses() call.
+   */
   fileList?: FileListInfo;
+  /**
+   * The branch from which the PR is created.
+   */
   sourceBranch: string;
+  /**
+   * The branch into which the PR is merged.
+   */
   targetBranch: string;
+  /**
+   * The SHA of the commit that is being merged in the PR. Originates from sourceBranch.
+   */
   sha: string;
+  /**
+   * The repository where the PR is created.
+   */
   repo: string;
+  /**
+   * The owner of the repository where the PR is created.
+   */
   owner: string;
+  /**
+   * The pull request number.
+   */
   prNumber: string;
+  /**
+   * Is the PR a draft PR?
+   */
   isDraft: boolean;
+
+  /**
+   * The context for labels applied to the PR. This context is passed throughout the labeling actions, and
+   * is used to determine which labels should be added or removed at the end of the labeling process.
+   */
   labelContext: LabelContext;
 
   constructor(
@@ -87,7 +140,6 @@
     return changedFiles;
   }
 
-  // todo get rid of async here not necessary
   // todo store the results
   getTypeSpecDiffs(): DiffResult<string> {
     if (!this.fileList) {
@@ -272,12 +324,6 @@
   // this function is based upon LocalDirContext.getReadmeDiffs() and CommonPRContext.getReadmeDiffs() which are
   // very different from each other (because why not). This implementation is based MOSTLY on CommonPRContext
   async getReadmeDiffs(): Promise<DiffResult<ReadmeTag>> {
-    // this gets all of the readme diffs
-    // const readmeAdditions = this.fileList?.additions.filter(file => readme(file)) || [];
-    // const readmeChanges = this.fileList?.modifications.filter(file => readme(file))
-    // const readmeDeletions = this.fileList?.deletions.filter(file => readme(file));
-    //const changedFiles: DiffFileResult | undefined = await this.localPRContext?.getChangingFiles();
-
     const changedFiles = await this.fileList;
     const tagDiffs = (await this.getChangingTags()) || [];
 

eng/tools/summarize-impact/src/impact.ts

@@ -1,3 +1,3 @@
 #!/usr/bin/env node
 
 import * as fs from "fs";
@@ -10,6 +10,8 @@
 
 import { breakingChangesCheckType } from "@azure-tools/specs-shared/breaking-change";
 
+import { dataPlane, resourceManager } from "@azure-tools/specs-shared/changed-files";
+
 import {
   DiffResult,
   ReadmeTag,
@@ -169,11 +171,11 @@
 }
 
 export function isManagementPR(filePaths: string[]): boolean {
-  return filePaths.some((it) => it.includes("resource-manager"));
+  return filePaths.some((it) => resourceManager(it));
 }
 
 export function isDataPlanePR(filePaths: string[]): boolean {
-  return filePaths.some((it) => it.includes("data-plane"));
+  return filePaths.some((it) => dataPlane(it));
 }
 
 export function getAllApiVersionFromRPFolder(rpFolder: string): string[] {