From 654b09447ad2917ec69b9541178a1efff508c339 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 9 Jul 2025 16:09:56 +0200 Subject: [PATCH 01/10] feat: optimize changelog check for package.json changes - Skip changelog when only version, dev dependencies, or both are changed --- src/changelog-check.ts | 216 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 191 insertions(+), 25 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index 1de17f25..d269021f 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -6,6 +6,7 @@ import path from 'path'; type PackageJson = { workspaces: string[]; private?: boolean; + version: string; }; /** @@ -111,31 +112,90 @@ async function getChangedFiles( } /** - * Checks if a package.json file only has version changes by comparing the diff output. - * Returns true if the diff contains exactly two lines (one addition and one removal) - * and both lines are version changes with the same format (with or without trailing comma). + * Checks if a specific change line is in the devDependencies section. + * + * @param changeLine - The specific change line to check. + * @param diffOutput - The full diff output for context. + * @returns True if the line is in devDependencies, false otherwise. + */ +const isLineInDevDependencies = ( + changeLine: string, + diffOutput: string, +): boolean => { + const allLines = diffOutput.split('\n'); + const changeLineIndex = allLines.findIndex((line) => line === changeLine); + + if (changeLineIndex === -1) { + return false; + } + + // Look backwards from the change line to find the nearest section header + for (let i = changeLineIndex; i >= 0; i--) { + const line = allLines[i]; + + if (!line) { + continue; + } + + // If we find devDependencies section, we're in it + if (line.includes('"devDependencies"') && line.includes(':')) { + return true; + } + + // If we find any other section first, we're not in devDependencies + if ( + (line.includes('"dependencies"') || + line.includes('"peerDependencies"') || + line.includes('"scripts"') || + line.includes('"engines"') || + line.includes('"main"') || + line.includes('"types"') || + line.includes('"files"')) && + line.includes(':') + ) { + return false; + } + } + + return false; +}; + +/** + * Analyzes all changes in a package.json file and returns structured information. * * @param repoPath - The path to the repository. * @param filePath - The path to the package.json file. * @param baseRef - The base reference to compare against. - * @returns Promise that resolves to true if only version was changed, false otherwise. + * @returns Promise that resolves to an object with all change information. */ -async function isVersionOnlyChange( +async function analyzePackageJsonChanges( repoPath: string, filePath: string, baseRef: string, -): Promise { +): Promise<{ + hasChanges: boolean; + isVersionOnly: boolean; + isDevDependencyOnly: boolean; + isVersionAndDevDependencyOnly: boolean; + newVersion: string | null; +}> { try { const { stdout } = await execa( 'git', - ['diff', `origin/${baseRef}...HEAD`, '--', filePath], + ['diff', '-U20', `origin/${baseRef}...HEAD`, '--', filePath], { cwd: repoPath, }, ); if (!stdout) { - return false; + return { + hasChanges: false, + isVersionOnly: false, + isDevDependencyOnly: false, + isVersionAndDevDependencyOnly: false, + newVersion: null, + }; } // Split the diff into lines and filter out the diff header lines (+++ and ---) @@ -144,20 +204,62 @@ async function isVersionOnlyChange( .filter((line) => line.startsWith('+') || line.startsWith('-')) .filter((line) => !line.startsWith('+++') && !line.startsWith('---')); - // If we have exactly 2 lines (one addition and one removal) and they both contain version changes - if (lines.length === 2) { - const versionRegex = /^[+-]\s*"version":\s*"[^"]+"\s*,?\s*$/mu; - return lines.every((line) => versionRegex.test(line)); + if (lines.length === 0) { + return { + hasChanges: false, + isVersionOnly: false, + isDevDependencyOnly: false, + isVersionAndDevDependencyOnly: false, + newVersion: null, + }; } - return false; + const versionAddedMatch = stdout.match(/^\+\s*"version":\s*"([^"]+)"/mu); + const newVersion = versionAddedMatch?.[1] ?? null; + const hasVersionChange = newVersion !== null; + + // Check if only version was changed (exactly 2 lines: one addition, one removal) + if (lines.length === 2 && hasVersionChange) { + return { + hasChanges: true, + isVersionOnly: true, + isDevDependencyOnly: false, + isVersionAndDevDependencyOnly: false, + newVersion, + }; + } + + // Filter out version lines to check the rest + const nonVersionLines = lines.filter( + (line) => !/^[+-]\s*"version":\s*"[^"]+"\s*,?\s*$/mu.test(line), + ); + + // Check if all non-version lines are in devDependencies + const allNonVersionLinesAreDevDeps = nonVersionLines.every((line) => + isLineInDevDependencies(line, stdout), + ); + + return { + hasChanges: true, + isVersionOnly: false, + isDevDependencyOnly: !hasVersionChange && allNonVersionLinesAreDevDeps, + isVersionAndDevDependencyOnly: + hasVersionChange && allNonVersionLinesAreDevDeps, + newVersion, + }; } catch (error) { logError( - `Failed to check ${filePath} changes: ${ + `Failed to analyze package.json changes in ${filePath}: ${ error instanceof Error ? error.message : String(error) }`, ); - return false; + return { + hasChanges: false, + isVersionOnly: false, + isDevDependencyOnly: false, + isVersionAndDevDependencyOnly: false, + newVersion: null, + }; } } @@ -194,10 +296,12 @@ async function isPrivatePackage( * * @param changelogPath - The path to the changelog file to check. * @param prNumber - The pull request number. + * @param packageVersion - The package version to check for release PRs. */ async function checkChangelogFile( changelogPath: string, prNumber: string, + packageVersion?: string | null, ): Promise { try { const changelogContent = await fs.readFile(changelogPath, 'utf-8'); @@ -206,18 +310,36 @@ async function checkChangelogFile( throw new Error('CHANGELOG.md is empty or missing'); } - const changelogUnreleasedChanges = parseChangelog({ + const changelogData = parseChangelog({ changelogContent, - repoUrl: '', // Not needed as we're only parsing unreleased changes - }).getReleaseChanges('Unreleased'); + repoUrl: '', // Not needed as we're only parsing changes + }); + + // For release PRs with version changes, check the version section + // Otherwise, check the Unreleased section + let releaseSection = 'Unreleased'; + if (packageVersion) { + // Check if this might be a release PR by looking for the version in changelog + try { + const versionChanges = changelogData.getReleaseChanges(packageVersion); + if (versionChanges && Object.keys(versionChanges).length > 0) { + releaseSection = packageVersion; + } + } catch { + // If version section doesn't exist, fallback to Unreleased + releaseSection = 'Unreleased'; + } + } + + const changelogChanges = changelogData.getReleaseChanges(releaseSection); if ( - !Object.values(changelogUnreleasedChanges) + !Object.values(changelogChanges) .flat() .some((entry) => entry.includes(`[#${prNumber}]`)) ) { throw new Error( - "This PR contains changes that might require documentation in the changelog. If these changes aren't user-facing, consider adding the 'no-changelog' label instead.", + "One or more changelogs might be out of date. If the changes you've introduced to a package are user-facing, please update its changelog by adding one or more entries for the changes to the Unreleased section, making sure to link the entries to the current PR. If your changes are not user-facing, you can bypass this check by adding the 'no-changelog' label to the PR.", ); } } catch (error) { @@ -246,9 +368,13 @@ async function getChangedPackages( { base: string; package: string; + newVersion?: string; }[] > { - const changedPackages = new Map(); + const changedPackages = new Map< + string, + { base: string; package: string; newVersion?: string } + >(); const privatePackageCache = new Map(); for (const file of files) { @@ -281,18 +407,57 @@ async function getChangedPackages( !file.includes('/docs/') && !file.endsWith('CHANGELOG.md') ) { - // If the file is package.json, check if it's only a version change + let newVersion: string | undefined; + if (file.endsWith('package.json')) { - const isVersionOnly = await isVersionOnlyChange( + const packageJsonChanges = await analyzePackageJsonChanges( repoPath, file, baseRef, ); - if (isVersionOnly) { + + if (!packageJsonChanges.hasChanges) { + continue; + } + + if (packageJsonChanges.newVersion) { + newVersion = packageJsonChanges.newVersion; + console.log( + `Detected version change to ${packageJsonChanges.newVersion} in ${packageInfo.package}`, + ); + } + + if (packageJsonChanges.isVersionOnly) { + console.log( + `Skipping package.json in ${packageInfo.package} as it only contains version changes`, + ); + continue; + } + + if (packageJsonChanges.isDevDependencyOnly) { + console.log( + `Skipping package.json in ${packageInfo.package} as it only contains dev dependency changes`, + ); + continue; + } + + if (packageJsonChanges.isVersionAndDevDependencyOnly) { + console.log( + `Skipping package.json in ${packageInfo.package} as it only contains version and dev dependency changes`, + ); continue; } } - changedPackages.set(packageInfo.package, packageInfo); + + const existingPackage = changedPackages.get(packageInfo.package); + const packageData = { + ...packageInfo, + ...(newVersion ? { newVersion } : {}), + ...(existingPackage?.newVersion + ? { newVersion: existingPackage.newVersion } + : {}), + }; + changedPackages.set(packageInfo.package, packageData); } } } @@ -361,6 +526,7 @@ async function main() { 'CHANGELOG.md', ), prNumber, + pkgInfo.newVersion, ); console.log( `CHANGELOG.md for ${pkgInfo.package} has been correctly updated.`, From ffabc9f2718a3b4481cad32ddddd29ac8de5eb43 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 9 Jul 2025 23:27:08 +0200 Subject: [PATCH 02/10] fix: address first loop of feedback --- src/changelog-check.ts | 62 ++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index d269021f..eab2139f 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -214,7 +214,24 @@ async function analyzePackageJsonChanges( }; } - const versionAddedMatch = stdout.match(/^\+\s*"version":\s*"([^"]+)"/mu); + const versionLines: string[] = []; + const nonVersionLines: string[] = []; + + for (const line of lines) { + if (/^[+-]\s*"version":\s*"[^"]+"\s*,?\s*$/mu.test(line)) { + versionLines.push(line); + } else { + nonVersionLines.push(line); + } + } + + // Check for version changes first + const versionAddedLine = versionLines.find( + (line) => line.startsWith('+') && line.includes('"version":'), + ); + const versionAddedMatch = versionAddedLine?.match( + /^\+\s*"version":\s*"([^"]+)"/u, + ); const newVersion = versionAddedMatch?.[1] ?? null; const hasVersionChange = newVersion !== null; @@ -229,11 +246,6 @@ async function analyzePackageJsonChanges( }; } - // Filter out version lines to check the rest - const nonVersionLines = lines.filter( - (line) => !/^[+-]\s*"version":\s*"[^"]+"\s*,?\s*$/mu.test(line), - ); - // Check if all non-version lines are in devDependencies const allNonVersionLinesAreDevDeps = nonVersionLines.every((line) => isLineInDevDependencies(line, stdout), @@ -319,15 +331,13 @@ async function checkChangelogFile( // Otherwise, check the Unreleased section let releaseSection = 'Unreleased'; if (packageVersion) { - // Check if this might be a release PR by looking for the version in changelog - try { - const versionChanges = changelogData.getReleaseChanges(packageVersion); - if (versionChanges && Object.keys(versionChanges).length > 0) { - releaseSection = packageVersion; - } - } catch { - // If version section doesn't exist, fallback to Unreleased - releaseSection = 'Unreleased'; + const versionChanges = changelogData.getReleaseChanges(packageVersion); + if (versionChanges) { + releaseSection = packageVersion; + } else { + throw new Error( + `Could not find section for version '${packageVersion}' in changelog`, + ); } } @@ -339,7 +349,7 @@ async function checkChangelogFile( .some((entry) => entry.includes(`[#${prNumber}]`)) ) { throw new Error( - "One or more changelogs might be out of date. If the changes you've introduced to a package are user-facing, please update its changelog by adding one or more entries for the changes to the Unreleased section, making sure to link the entries to the current PR. If your changes are not user-facing, you can bypass this check by adding the 'no-changelog' label to the PR.", + `There are changes made to this package that may not be reflected in the changelog ("${changelogPath}"). If the changes you've introduced are user-facing, please document them under the "${releaseSection}" section, making sure to link the entries to the current PR. If the changelog is up to date, you can bypass this check by adding the 'no-changelog' label to the PR.`, ); } } catch (error) { @@ -368,12 +378,12 @@ async function getChangedPackages( { base: string; package: string; - newVersion?: string; + newVersion?: string | undefined; }[] > { const changedPackages = new Map< string, - { base: string; package: string; newVersion?: string } + { base: string; package: string; newVersion?: string | undefined } >(); const privatePackageCache = new Map(); @@ -420,13 +430,6 @@ async function getChangedPackages( continue; } - if (packageJsonChanges.newVersion) { - newVersion = packageJsonChanges.newVersion; - console.log( - `Detected version change to ${packageJsonChanges.newVersion} in ${packageInfo.package}`, - ); - } - if (packageJsonChanges.isVersionOnly) { console.log( `Skipping package.json in ${packageInfo.package} as it only contains version changes`, @@ -447,15 +450,16 @@ async function getChangedPackages( ); continue; } + + if (packageJsonChanges.newVersion) { + newVersion = packageJsonChanges.newVersion; + } } const existingPackage = changedPackages.get(packageInfo.package); const packageData = { ...packageInfo, - ...(newVersion ? { newVersion } : {}), - ...(existingPackage?.newVersion - ? { newVersion: existingPackage.newVersion } - : {}), + newVersion: existingPackage?.newVersion ?? newVersion, }; changedPackages.set(packageInfo.package, packageData); } From 477df97f9d9d84bc85885d274a03f63f5053b87b Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 9 Jul 2025 23:46:34 +0200 Subject: [PATCH 03/10] fix: improve dev deps lookup perfs --- src/changelog-check.ts | 91 ++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index eab2139f..0b08b0fd 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -112,52 +112,67 @@ async function getChangedFiles( } /** - * Checks if a specific change line is in the devDependencies section. + * Extracts lines that are within devDependencies sections from the diff output. * - * @param changeLine - The specific change line to check. - * @param diffOutput - The full diff output for context. - * @returns True if the line is in devDependencies, false otherwise. + * @param diffOutput - The full diff output. + * @param nonVersionLines - The non-version change lines to filter. + * @returns Array of lines that are within devDependencies sections. */ -const isLineInDevDependencies = ( - changeLine: string, +const getDevDependencyLines = ( diffOutput: string, -): boolean => { + nonVersionLines: string[], +): string[] => { const allLines = diffOutput.split('\n'); - const changeLineIndex = allLines.findIndex((line) => line === changeLine); + const devDependencyLines: string[] = []; - if (changeLineIndex === -1) { - return false; - } + const devDepSectionBoundaries: { start: number; end: number }[] = []; - // Look backwards from the change line to find the nearest section header - for (let i = changeLineIndex; i >= 0; i--) { + for (let i = 0; i < allLines.length; i++) { const line = allLines[i]; - if (!line) { - continue; - } + if (line?.includes('"devDependencies"') && line.includes(':')) { + const startIndex = i; + let endIndex = allLines.length - 1; + + // Find the end of this section (next section or closing brace) + for (let j = i + 1; j < allLines.length; j++) { + const nextLine = allLines[j]; + if ( + nextLine && + (nextLine.includes('"dependencies"') || + nextLine.includes('"peerDependencies"') || + nextLine.includes('"scripts"') || + nextLine.includes('"engines"') || + nextLine.includes('"main"') || + nextLine.includes('"types"') || + nextLine.includes('"files"')) && + nextLine.includes(':') + ) { + endIndex = j - 1; + break; + } + } - // If we find devDependencies section, we're in it - if (line.includes('"devDependencies"') && line.includes(':')) { - return true; + devDepSectionBoundaries.push({ start: startIndex, end: endIndex }); } + } - // If we find any other section first, we're not in devDependencies - if ( - (line.includes('"dependencies"') || - line.includes('"peerDependencies"') || - line.includes('"scripts"') || - line.includes('"engines"') || - line.includes('"main"') || - line.includes('"types"') || - line.includes('"files"')) && - line.includes(':') - ) { - return false; + // Check which nonVersionLines fall within devDependencies sections + for (const changeLine of nonVersionLines) { + const lineIndex = allLines.findIndex((line) => line === changeLine); + if (lineIndex !== -1) { + // Check if this line falls within any devDependencies section + const isInDevDeps = devDepSectionBoundaries.some( + (section) => lineIndex >= section.start && lineIndex <= section.end, + ); + + if (isInDevDeps) { + devDependencyLines.push(changeLine); + } } } - return false; + return devDependencyLines; }; /** @@ -235,8 +250,8 @@ async function analyzePackageJsonChanges( const newVersion = versionAddedMatch?.[1] ?? null; const hasVersionChange = newVersion !== null; - // Check if only version was changed (exactly 2 lines: one addition, one removal) - if (lines.length === 2 && hasVersionChange) { + // Check if only version was changed + if (nonVersionLines.length === 0) { return { hasChanges: true, isVersionOnly: true, @@ -247,9 +262,9 @@ async function analyzePackageJsonChanges( } // Check if all non-version lines are in devDependencies - const allNonVersionLinesAreDevDeps = nonVersionLines.every((line) => - isLineInDevDependencies(line, stdout), - ); + const devDependencyLines = getDevDependencyLines(stdout, nonVersionLines); + const allNonVersionLinesAreDevDeps = + devDependencyLines.length === nonVersionLines.length; return { hasChanges: true, @@ -332,7 +347,7 @@ async function checkChangelogFile( let releaseSection = 'Unreleased'; if (packageVersion) { const versionChanges = changelogData.getReleaseChanges(packageVersion); - if (versionChanges) { + if (versionChanges && Object.keys(versionChanges).length > 0) { releaseSection = packageVersion; } else { throw new Error( From 31d4841a0b8f24db2155ab68493f2518ddd2dc31 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 10 Jul 2025 11:20:47 +0200 Subject: [PATCH 04/10] fix: handle revert release case --- package.json | 2 ++ src/changelog-check.ts | 54 +++++++++++++++++++++++++++++++++++++++--- yarn.lock | 18 ++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 575c226f..fe6b2081 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "googleapis": "144.0.0", "luxon": "^3.3.0", "ora": "^5.4.1", + "semver": "^7.6.3", "simple-git": "3.27.0" }, "devDependencies": { @@ -47,6 +48,7 @@ "@swc/core": "^1.3.80", "@types/jest": "^28.1.6", "@types/node": "^20.3.2", + "@types/semver": "^7", "@typescript-eslint/eslint-plugin": "^5.43.0", "@typescript-eslint/parser": "^5.43.0", "depcheck": "^1.4.3", diff --git a/src/changelog-check.ts b/src/changelog-check.ts index 0b08b0fd..0d2aa248 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -2,6 +2,7 @@ import { parseChangelog } from '@metamask/auto-changelog'; import { execa } from 'execa'; import fs from 'fs/promises'; import path from 'path'; +import { SemVer } from 'semver'; type PackageJson = { workspaces: string[]; @@ -175,6 +176,20 @@ const getDevDependencyLines = ( return devDependencyLines; }; +/** + * Checks if a version change is a downgrade (revert). + * + * @param oldVersion - The old version from the diff. + * @param newVersion - The new version from the diff. + * @returns True if this is a version downgrade, false otherwise. + */ +const isVersionDowngrade = ( + oldVersion: string, + newVersion: string, +): boolean => { + return new SemVer(newVersion).compare(new SemVer(oldVersion)) <= 0; +}; + /** * Analyzes all changes in a package.json file and returns structured information. * @@ -192,6 +207,7 @@ async function analyzePackageJsonChanges( isVersionOnly: boolean; isDevDependencyOnly: boolean; isVersionAndDevDependencyOnly: boolean; + isVersionDowngrade: boolean; newVersion: string | null; }> { try { @@ -209,6 +225,7 @@ async function analyzePackageJsonChanges( isVersionOnly: false, isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, + isVersionDowngrade: false, newVersion: null, }; } @@ -225,6 +242,7 @@ async function analyzePackageJsonChanges( isVersionOnly: false, isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, + isVersionDowngrade: false, newVersion: null, }; } @@ -248,7 +266,27 @@ async function analyzePackageJsonChanges( /^\+\s*"version":\s*"([^"]+)"/u, ); const newVersion = versionAddedMatch?.[1] ?? null; - const hasVersionChange = newVersion !== null; + + const versionRemovedLine = versionLines.find( + (line) => line.startsWith('-') && line.includes('"version":'), + ); + const versionRemovedMatch = versionRemovedLine?.match( + /^-\s*"version":\s*"([^"]+)"/u, + ); + const oldVersion = versionRemovedMatch?.[1] ?? null; + + const hasNewVersion = newVersion !== null; + + if (!hasNewVersion && oldVersion) { + throw new Error( + `Could not find new version for version change in ${filePath}`, + ); + } + + const isDowngrade = + oldVersion && newVersion + ? isVersionDowngrade(oldVersion, newVersion) + : false; // Check if only version was changed if (nonVersionLines.length === 0) { @@ -257,6 +295,7 @@ async function analyzePackageJsonChanges( isVersionOnly: true, isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, + isVersionDowngrade: isDowngrade, newVersion, }; } @@ -269,9 +308,10 @@ async function analyzePackageJsonChanges( return { hasChanges: true, isVersionOnly: false, - isDevDependencyOnly: !hasVersionChange && allNonVersionLinesAreDevDeps, + isDevDependencyOnly: !hasNewVersion && allNonVersionLinesAreDevDeps, isVersionAndDevDependencyOnly: - hasVersionChange && allNonVersionLinesAreDevDeps, + hasNewVersion && allNonVersionLinesAreDevDeps, + isVersionDowngrade: isDowngrade, newVersion, }; } catch (error) { @@ -285,6 +325,7 @@ async function analyzePackageJsonChanges( isVersionOnly: false, isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, + isVersionDowngrade: false, newVersion: null, }; } @@ -452,6 +493,13 @@ async function getChangedPackages( continue; } + if (packageJsonChanges.isVersionDowngrade) { + console.log( + `Skipping package.json in ${packageInfo.package} as it contains a version downgrade (revert)`, + ); + continue; + } + if (packageJsonChanges.isDevDependencyOnly) { console.log( `Skipping package.json in ${packageInfo.package} as it only contains dev dependency changes`, diff --git a/yarn.lock b/yarn.lock index 108a0de9..e85a144d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1027,6 +1027,7 @@ __metadata: "@types/jest": "npm:^28.1.6" "@types/luxon": "npm:^3.3.0" "@types/node": "npm:^20.3.2" + "@types/semver": "npm:^7" "@typescript-eslint/eslint-plugin": "npm:^5.43.0" "@typescript-eslint/parser": "npm:^5.43.0" axios: "npm:^0.24.0" @@ -1048,6 +1049,7 @@ __metadata: ora: "npm:^5.4.1" prettier: "npm:^2.7.1" prettier-plugin-packagejson: "npm:^2.3.0" + semver: "npm:^7.6.3" simple-git: "npm:3.27.0" ts-jest: "npm:^28.0.7" ts-node: "npm:^10.9.1" @@ -1919,6 +1921,13 @@ __metadata: languageName: node linkType: hard +"@types/semver@npm:^7": + version: 7.7.0 + resolution: "@types/semver@npm:7.7.0" + checksum: 10/ee4514c6c852b1c38f951239db02f9edeea39f5310fad9396a00b51efa2a2d96b3dfca1ae84c88181ea5b7157c57d32d7ef94edacee36fbf975546396b85ba5b + languageName: node + linkType: hard + "@types/semver@npm:^7.3.12": version: 7.3.13 resolution: "@types/semver@npm:7.3.13" @@ -7259,6 +7268,15 @@ __metadata: languageName: node linkType: hard +"semver@npm:^7.6.3": + version: 7.7.2 + resolution: "semver@npm:7.7.2" + bin: + semver: bin/semver.js + checksum: 10/7a24cffcaa13f53c09ce55e05efe25cd41328730b2308678624f8b9f5fc3093fc4d189f47950f0b811ff8f3c3039c24a2c36717ba7961615c682045bf03e1dda + languageName: node + linkType: hard + "set-blocking@npm:^2.0.0": version: 2.0.0 resolution: "set-blocking@npm:2.0.0" From aa901a61ee2a58807a79e99e80930614d3990ec9 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 10 Jul 2025 11:24:36 +0200 Subject: [PATCH 05/10] fix: semver version --- package.json | 2 +- yarn.lock | 39 ++++++--------------------------------- 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index fe6b2081..9285b795 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "googleapis": "144.0.0", "luxon": "^3.3.0", "ora": "^5.4.1", - "semver": "^7.6.3", + "semver": "^7.7.2", "simple-git": "3.27.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index e85a144d..7e92bef3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1049,7 +1049,7 @@ __metadata: ora: "npm:^5.4.1" prettier: "npm:^2.7.1" prettier-plugin-packagejson: "npm:^2.3.0" - semver: "npm:^7.6.3" + semver: "npm:^7.7.2" simple-git: "npm:3.27.0" ts-jest: "npm:^28.0.7" ts-node: "npm:^10.9.1" @@ -1921,20 +1921,13 @@ __metadata: languageName: node linkType: hard -"@types/semver@npm:^7": +"@types/semver@npm:^7, @types/semver@npm:^7.3.12": version: 7.7.0 resolution: "@types/semver@npm:7.7.0" checksum: 10/ee4514c6c852b1c38f951239db02f9edeea39f5310fad9396a00b51efa2a2d96b3dfca1ae84c88181ea5b7157c57d32d7ef94edacee36fbf975546396b85ba5b languageName: node linkType: hard -"@types/semver@npm:^7.3.12": - version: 7.3.13 - resolution: "@types/semver@npm:7.3.13" - checksum: 10/0064efd7a0515a539062b71630c72ca2b058501b957326c285cdff82f42c1716d9f9f831332ccf719d5ee8cc3ef24f9ff62122d7a7140c73959a240b49b0f62d - languageName: node - linkType: hard - "@types/stack-utils@npm:^2.0.0": version: 2.0.0 resolution: "@types/stack-utils@npm:2.0.0" @@ -5956,15 +5949,6 @@ __metadata: languageName: node linkType: hard -"lru-cache@npm:^6.0.0": - version: 6.0.0 - resolution: "lru-cache@npm:6.0.0" - dependencies: - yallist: "npm:^4.0.0" - checksum: 10/fc1fe2ee205f7c8855fa0f34c1ab0bcf14b6229e35579ec1fd1079f31d6fc8ef8eb6fd17f2f4d99788d7e339f50e047555551ebd5e434dda503696e7c6591825 - languageName: node - linkType: hard - "lru-cache@npm:^7.7.1": version: 7.13.1 resolution: "lru-cache@npm:7.13.1" @@ -7248,14 +7232,12 @@ __metadata: languageName: node linkType: hard -"semver@npm:7.x, semver@npm:^7.0.0, semver@npm:^7.3.2, semver@npm:^7.3.5, semver@npm:^7.3.7, semver@npm:^7.3.8, semver@npm:^7.5.3, semver@npm:^7.5.4": - version: 7.5.4 - resolution: "semver@npm:7.5.4" - dependencies: - lru-cache: "npm:^6.0.0" +"semver@npm:7.x, semver@npm:^7.0.0, semver@npm:^7.3.2, semver@npm:^7.3.5, semver@npm:^7.3.7, semver@npm:^7.3.8, semver@npm:^7.5.3, semver@npm:^7.5.4, semver@npm:^7.7.2": + version: 7.7.2 + resolution: "semver@npm:7.7.2" bin: semver: bin/semver.js - checksum: 10/985dec0d372370229a262c737063860fabd4a1c730662c1ea3200a2f649117761a42184c96df62a0e885e76fbd5dace41087d6c1ac0351b13c0df5d6bcb1b5ac + checksum: 10/7a24cffcaa13f53c09ce55e05efe25cd41328730b2308678624f8b9f5fc3093fc4d189f47950f0b811ff8f3c3039c24a2c36717ba7961615c682045bf03e1dda languageName: node linkType: hard @@ -7268,15 +7250,6 @@ __metadata: languageName: node linkType: hard -"semver@npm:^7.6.3": - version: 7.7.2 - resolution: "semver@npm:7.7.2" - bin: - semver: bin/semver.js - checksum: 10/7a24cffcaa13f53c09ce55e05efe25cd41328730b2308678624f8b9f5fc3093fc4d189f47950f0b811ff8f3c3039c24a2c36717ba7961615c682045bf03e1dda - languageName: node - linkType: hard - "set-blocking@npm:^2.0.0": version: 2.0.0 resolution: "set-blocking@npm:2.0.0" From 6e40a90c22fb431385ab12ed6bd4dd719d6bfdfa Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 10 Jul 2025 11:31:30 +0200 Subject: [PATCH 06/10] fix: version downgrade check --- src/changelog-check.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index 0d2aa248..b6e58e29 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -187,7 +187,7 @@ const isVersionDowngrade = ( oldVersion: string, newVersion: string, ): boolean => { - return new SemVer(newVersion).compare(new SemVer(oldVersion)) <= 0; + return new SemVer(newVersion).compare(new SemVer(oldVersion)) < 0; }; /** From b5dc65740486bb03c103103bf8f70df15d4fd6b4 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 10 Jul 2025 14:15:29 +0200 Subject: [PATCH 07/10] refactor: extract changelog path --- src/changelog-check.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index b6e58e29..0efa00ef 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -392,7 +392,7 @@ async function checkChangelogFile( releaseSection = packageVersion; } else { throw new Error( - `Could not find section for version '${packageVersion}' in changelog`, + `Could not find section for version '${packageVersion}' in the changelog ("${changelogPath}").`, ); } } @@ -585,16 +585,13 @@ async function main() { const checkResults = await Promise.all( changedPackages.map(async (pkgInfo) => { try { - await checkChangelogFile( - path.join( - fullRepoPath, - pkgInfo.base, - pkgInfo.package, - 'CHANGELOG.md', - ), - prNumber, - pkgInfo.newVersion, + const changelogPath = path.join( + fullRepoPath, + pkgInfo.base, + pkgInfo.package, + 'CHANGELOG.md', ); + await checkChangelogFile(changelogPath, prNumber, pkgInfo.newVersion); console.log( `CHANGELOG.md for ${pkgInfo.package} has been correctly updated.`, ); From fb3a13fbba7fede685d15d03620755aa112125bd Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 10 Jul 2025 18:49:02 +0200 Subject: [PATCH 08/10] fix: address PR feedback loop 2 --- src/changelog-check.ts | 100 ++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 62 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index 0efa00ef..464c5d4c 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -126,46 +126,35 @@ const getDevDependencyLines = ( const allLines = diffOutput.split('\n'); const devDependencyLines: string[] = []; - const devDepSectionBoundaries: { start: number; end: number }[] = []; + let devDependencySectionStart: number | undefined; + let devDependencySectionEnd: number | undefined; for (let i = 0; i < allLines.length; i++) { - const line = allLines[i]; - - if (line?.includes('"devDependencies"') && line.includes(':')) { - const startIndex = i; - let endIndex = allLines.length - 1; - - // Find the end of this section (next section or closing brace) - for (let j = i + 1; j < allLines.length; j++) { - const nextLine = allLines[j]; - if ( - nextLine && - (nextLine.includes('"dependencies"') || - nextLine.includes('"peerDependencies"') || - nextLine.includes('"scripts"') || - nextLine.includes('"engines"') || - nextLine.includes('"main"') || - nextLine.includes('"types"') || - nextLine.includes('"files"')) && - nextLine.includes(':') - ) { - endIndex = j - 1; - break; - } - } + const line = allLines[i] as string; - devDepSectionBoundaries.push({ start: startIndex, end: endIndex }); + if (line.includes('"devDependencies":')) { + devDependencySectionStart = i; + } else if (devDependencySectionStart && /^\s*\}/u.test(line)) { + devDependencySectionEnd = i; + break; } } + if ( + devDependencySectionStart === undefined || + devDependencySectionEnd === undefined + ) { + return []; + } + // Check which nonVersionLines fall within devDependencies sections for (const changeLine of nonVersionLines) { const lineIndex = allLines.findIndex((line) => line === changeLine); if (lineIndex !== -1) { // Check if this line falls within any devDependencies section - const isInDevDeps = devDepSectionBoundaries.some( - (section) => lineIndex >= section.start && lineIndex <= section.end, - ); + const isInDevDeps = + lineIndex >= devDependencySectionStart && + lineIndex <= devDependencySectionEnd; if (isInDevDeps) { devDependencyLines.push(changeLine); @@ -208,7 +197,7 @@ async function analyzePackageJsonChanges( isDevDependencyOnly: boolean; isVersionAndDevDependencyOnly: boolean; isVersionDowngrade: boolean; - newVersion: string | null; + newVersion: string | undefined; }> { try { const { stdout } = await execa( @@ -226,7 +215,7 @@ async function analyzePackageJsonChanges( isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, isVersionDowngrade: false, - newVersion: null, + newVersion: undefined, }; } @@ -243,44 +232,31 @@ async function analyzePackageJsonChanges( isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, isVersionDowngrade: false, - newVersion: null, + newVersion: undefined, }; } - const versionLines: string[] = []; + const versionLines: { type: 'added' | 'removed'; version: string }[] = []; const nonVersionLines: string[] = []; + let oldVersion: string | undefined; + let newVersion: string | undefined; for (const line of lines) { - if (/^[+-]\s*"version":\s*"[^"]+"\s*,?\s*$/mu.test(line)) { - versionLines.push(line); + const match = line.match(/^([+-])\s*"version":\s*"([^"]+)"\s*,?\s*$/u); + if (match) { + const type = match[1] === '+' ? 'added' : 'removed'; + const version = match[2] as string; + versionLines.push({ type, version }); } else { nonVersionLines.push(line); } } - - // Check for version changes first - const versionAddedLine = versionLines.find( - (line) => line.startsWith('+') && line.includes('"version":'), - ); - const versionAddedMatch = versionAddedLine?.match( - /^\+\s*"version":\s*"([^"]+)"/u, - ); - const newVersion = versionAddedMatch?.[1] ?? null; - - const versionRemovedLine = versionLines.find( - (line) => line.startsWith('-') && line.includes('"version":'), - ); - const versionRemovedMatch = versionRemovedLine?.match( - /^-\s*"version":\s*"([^"]+)"/u, - ); - const oldVersion = versionRemovedMatch?.[1] ?? null; - - const hasNewVersion = newVersion !== null; - - if (!hasNewVersion && oldVersion) { - throw new Error( - `Could not find new version for version change in ${filePath}`, - ); + if ( + versionLines?.[0]?.type === 'removed' && + versionLines?.[1]?.type === 'added' + ) { + oldVersion = versionLines[0].version; + newVersion = versionLines[1].version; } const isDowngrade = @@ -308,9 +284,9 @@ async function analyzePackageJsonChanges( return { hasChanges: true, isVersionOnly: false, - isDevDependencyOnly: !hasNewVersion && allNonVersionLinesAreDevDeps, + isDevDependencyOnly: !newVersion && allNonVersionLinesAreDevDeps, isVersionAndDevDependencyOnly: - hasNewVersion && allNonVersionLinesAreDevDeps, + newVersion !== undefined && allNonVersionLinesAreDevDeps, isVersionDowngrade: isDowngrade, newVersion, }; @@ -326,7 +302,7 @@ async function analyzePackageJsonChanges( isDevDependencyOnly: false, isVersionAndDevDependencyOnly: false, isVersionDowngrade: false, - newVersion: null, + newVersion: undefined, }; } } From e268a9b82fff15627d3cd22bef78724d2c1cd6a2 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 11 Jul 2025 23:02:40 +0200 Subject: [PATCH 09/10] fix: remove useless optional chaining operator --- src/changelog-check.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index 464c5d4c..3282066b 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -252,8 +252,8 @@ async function analyzePackageJsonChanges( } } if ( - versionLines?.[0]?.type === 'removed' && - versionLines?.[1]?.type === 'added' + versionLines[0]?.type === 'removed' && + versionLines[1]?.type === 'added' ) { oldVersion = versionLines[0].version; newVersion = versionLines[1].version; From 5ad7e408674b3216fed31ed8e392dad5fe83a567 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 11 Jul 2025 23:37:08 +0200 Subject: [PATCH 10/10] fix: displayed changelog paths --- src/changelog-check.ts | 43 ++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/src/changelog-check.ts b/src/changelog-check.ts index 3282066b..822604c5 100644 --- a/src/changelog-check.ts +++ b/src/changelog-check.ts @@ -338,17 +338,28 @@ async function isPrivatePackage( /** * Reads and validates a changelog file. * - * @param changelogPath - The path to the changelog file to check. - * @param prNumber - The pull request number. - * @param packageVersion - The package version to check for release PRs. + * @param params - The parameters for the changelog file check. + * @param params.repoPath - The path to the repository. + * @param params.changelogPath - The path to the changelog file to check. + * @param params.prNumber - The pull request number. + * @param params.packageVersion - The package version to check for release PRs. */ -async function checkChangelogFile( - changelogPath: string, - prNumber: string, - packageVersion?: string | null, -): Promise { +async function checkChangelogFile({ + repoPath, + changelogPath, + prNumber, + packageVersion, +}: { + repoPath: string; + changelogPath: string; + prNumber: string; + packageVersion?: string | null; +}): Promise { try { - const changelogContent = await fs.readFile(changelogPath, 'utf-8'); + const changelogContent = await fs.readFile( + path.join(repoPath, changelogPath), + 'utf-8', + ); if (!changelogContent) { throw new Error('CHANGELOG.md is empty or missing'); @@ -562,12 +573,16 @@ async function main() { changedPackages.map(async (pkgInfo) => { try { const changelogPath = path.join( - fullRepoPath, pkgInfo.base, pkgInfo.package, 'CHANGELOG.md', ); - await checkChangelogFile(changelogPath, prNumber, pkgInfo.newVersion); + await checkChangelogFile({ + repoPath: fullRepoPath, + changelogPath, + prNumber, + packageVersion: pkgInfo.newVersion ?? null, + }); console.log( `CHANGELOG.md for ${pkgInfo.package} has been correctly updated.`, ); @@ -592,7 +607,11 @@ async function main() { console.log( 'Running in single-repo mode - checking changelog for the entire repository...', ); - await checkChangelogFile(path.join(fullRepoPath, 'CHANGELOG.md'), prNumber); + await checkChangelogFile({ + repoPath: fullRepoPath, + changelogPath: 'CHANGELOG.md', + prNumber, + }); console.log('CHANGELOG.md has been correctly updated.'); } }