Fixed PR Changelog Generation Workflow (#40410)

This adds support for using double-quotes in the
description of the PR.
This commit is contained in:
Christopher Allford 2023-09-26 14:26:12 -07:00 committed by GitHub
parent 67dadbb249
commit b6674ef0c1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 145 additions and 57 deletions

View File

@ -2,10 +2,9 @@
* External dependencies
*/
import { Command } from '@commander-js/extra-typings';
import { execSync } from 'child_process';
import simpleGit from 'simple-git';
import nodePath from 'path';
import { existsSync } from 'fs';
import { existsSync, readFileSync, rmSync, writeFileSync } from 'fs';
/**
* Internal dependencies
@ -118,60 +117,85 @@ const program = new Command( 'changefile' )
'Removing existing changelog files in case a change is reverted and the entry is no longer needed'
);
allProjectPaths.forEach( ( projectPath ) => {
const path = nodePath.join(
const composerFilePath = nodePath.join(
tmpRepoPath,
projectPath,
'changelog',
'composer.json'
);
if ( ! existsSync( composerFilePath ) ) {
return;
}
// Figure out where the changelog files belong for this project.
const composerFile = JSON.parse(
readFileSync( composerFilePath, {
encoding: 'utf-8',
} )
);
const changelogFilePath = nodePath.join(
tmpRepoPath,
projectPath,
composerFile.extra?.changelogger[ 'changes-dir' ] ??
'changelog',
fileName
);
if ( existsSync( path ) ) {
Logger.notice(
`Remove existing changelog file ${ path }`
);
execSync( `rm ${ path }`, {
cwd: tmpRepoPath,
stdio: 'inherit',
} );
if ( ! existsSync( changelogFilePath ) ) {
return;
}
Logger.notice(
`Remove existing changelog file ${ changelogFilePath }`
);
rmSync( changelogFilePath );
} );
if ( touchedProjectsRequiringChangelog.length === 0 ) {
if ( ! touchedProjectsRequiringChangelog ) {
Logger.notice( 'No projects require a changelog' );
process.exit( 0 );
}
// When a devRepoPath is provided, assume that the dependencies are already installed.
if ( ! devRepoPath ) {
Logger.notice(
`Installing dependencies in ${ tmpRepoPath }`
for ( const project in touchedProjectsRequiringChangelog ) {
const projectPath = nodePath.join(
tmpRepoPath,
touchedProjectsRequiringChangelog[ project ]
);
execSync( 'pnpm install', {
cwd: tmpRepoPath,
stdio: 'inherit',
} );
}
touchedProjectsRequiringChangelog.forEach( ( project ) => {
Logger.notice(
`Running changelog command for ${ project }`
`Generating changefile for ${ project } (${ projectPath }))`
);
const messageExpression = message
? `-e "${ message }"`
: '--entry=""';
const commentExpression = comment
? `-c "${ comment }"`
: '';
const cmd = `pnpm --filter=${ project } run changelog add -f ${ fileName } -s ${ significance } -t ${ type } ${ messageExpression } ${ commentExpression } -n`;
execSync( cmd, { cwd: tmpRepoPath, stdio: 'inherit' } );
} );
// Figure out where the changelog file belongs for this project.
const composerFile = JSON.parse(
readFileSync(
nodePath.join( projectPath, 'composer.json' ),
{ encoding: 'utf-8' }
)
);
const changelogFilePath = nodePath.join(
projectPath,
composerFile.extra?.changelogger[ 'changes-dir' ] ??
'changelog',
fileName
);
// Write the changefile using the correct format.
let fileContent = `Significance: ${ significance }\n`;
fileContent += `Type: ${ type }\n`;
if ( comment ) {
fileContent += `Comment: ${ comment }\n`;
}
fileContent += `\n${ message }`;
writeFileSync( changelogFilePath, fileContent );
}
} catch ( e ) {
Logger.error( e );
}
const touchedProjectsString =
touchedProjectsRequiringChangelog.join( ', ' );
const touchedProjectsString = Object.keys(
touchedProjectsRequiringChangelog
).join( ', ' );
Logger.notice(
`Changelogs created for ${ touchedProjectsString }`

View File

@ -351,6 +351,53 @@ describe( 'getChangelogDetails', () => {
expect( details.comment ).toEqual( 'This is a very useful comment.' );
} );
it( 'should remove newlines from message and comment', () => {
const body =
'### Changelog entry\r\n' +
'\r\n' +
'<!-- You can optionally choose to enter a changelog entry by checking the box and supplying data. -->\r\n' +
'\r\n' +
'- [x] Automatically create a changelog entry from the details below.\r\n' +
'\r\n' +
'<details>\r\n' +
'\r\n' +
'#### Significance\r\n' +
'<!-- Choose only one -->\r\n' +
'- [x] Patch\r\n' +
'- [ ] Minor\r\n' +
'- [ ] Major\r\n' +
'\r\n' +
'#### Type\r\n' +
'<!-- Choose only one -->\r\n' +
'- [x] Fix - Fixes an existing bug\r\n' +
'- [ ] Add - Adds functionality\r\n' +
'- [ ] Update - Update existing functionality\r\n' +
'- [ ] Dev - Development related task\r\n' +
'- [ ] Tweak - A minor adjustment to the codebase\r\n' +
'- [ ] Performance - Address performance issues\r\n' +
'- [ ] Enhancement\r\n' +
'\r\n' +
'#### Message ' +
'<!-- Add a changelog message here -->\r\n' +
'This is a very useful fix.\r\n' +
'I promise!\r\n' +
'\r\n' +
'#### Comment ' +
`<!-- If the changes in this pull request don't warrant a changelog entry, you can alternatively supply a comment here. Note that comments are only accepted with a significance of "Patch" -->\r\n` +
'This is a very useful comment.\r\n' +
"I don't promise!\r\n" +
'\r\n' +
'</details>';
const details = getChangelogDetails( body );
expect( details.message ).toEqual(
'This is a very useful fix. I promise!'
);
expect( details.comment ).toEqual(
"This is a very useful comment. I don't promise!"
);
} );
it( 'should return a comment even when it is entered with a significance other than patch', () => {
const body =
'### Changelog entry\r\n' +

View File

@ -93,11 +93,11 @@ describe( 'Changelog project functions', () => {
changeLoggerProjects
);
expect( intersectedProjects ).toHaveLength( 2 );
expect( intersectedProjects ).toContain(
'folder-with-lots-of-projects/project-b'
);
expect( intersectedProjects ).toContain( 'projects/very-cool-project' );
expect( intersectedProjects ).toMatchObject( {
'folder-with-lots-of-projects/project-b':
'folder-with-lots-of-projects/project-b',
'projects/very-cool-project': 'projects/very-cool-project',
} );
} );
it( 'getTouchedChangeloggerProjectsPathsMappedToProjects should map plugins and js packages to the correct name', async () => {
@ -119,11 +119,12 @@ describe( 'Changelog project functions', () => {
changeLoggerProjects
);
expect( intersectedProjects ).toHaveLength( 4 );
expect( intersectedProjects ).toContain( 'woocommerce' );
expect( intersectedProjects ).toContain( 'beta-tester' );
expect( intersectedProjects ).toContain( '@woocommerce/components' );
expect( intersectedProjects ).toContain( '@woocommerce/data' );
expect( intersectedProjects ).toMatchObject( {
woocommerce: 'plugins/woocommerce',
'beta-tester': 'plugins/beta-tester',
'@woocommerce/components': 'packages/js/components',
'@woocommerce/data': 'packages/js/data',
} );
} );
it( 'getTouchedChangeloggerProjectsPathsMappedToProjects should handle woocommerce-admin projects mapped to woocommerce core', async () => {
@ -138,7 +139,8 @@ describe( 'Changelog project functions', () => {
changeLoggerProjects
);
expect( intersectedProjects ).toHaveLength( 1 );
expect( intersectedProjects ).toContain( 'woocommerce' );
expect( intersectedProjects ).toMatchObject( {
woocommerce: 'plugins/woocommerce',
} );
} );
} );

View File

@ -120,7 +120,12 @@ export const getChangelogMessage = ( body: string ) => {
Logger.error( 'No changelog message found' );
}
return match[ 3 ].trim();
let message = match[ 3 ].trim();
// Newlines break the formatting of the changelog, so we replace them with spaces.
message = message.replace( /\r\n|\n/g, ' ' );
return message;
};
/**
@ -133,7 +138,12 @@ export const getChangelogComment = ( body: string ) => {
const commentRegex = /#### Comment ?(<!--(.*)-->)?(.*)<\/details>/gms;
const match = commentRegex.exec( body );
return match ? match[ 3 ].trim() : '';
let comment = match ? match[ 3 ].trim() : '';
// Newlines break the formatting of the changelog, so we replace them with spaces.
comment = comment.replace( /\r\n|\n/g, ' ' );
return comment;
};
/**

View File

@ -120,7 +120,7 @@ export const getTouchedFilePaths = async (
*
* @param {Array<string>} touchedFiles List of files changed in a PR. touchedFiles
* @param {Array<string>} changeloggerProjects List of projects that have Jetpack changelogger enabled.
* @return {Array<string>} List of projects that have Jetpack changelogger enabled and have files changed in a PR.
* @return {Object.<string, string>} Paths to projects that have files changed in a PR keyed by the project name.
*/
export const getTouchedChangeloggerProjectsPathsMappedToProjects = (
touchedFiles: Array< string >,
@ -142,14 +142,19 @@ export const getTouchedChangeloggerProjectsPathsMappedToProjects = (
);
}
);
return touchedProjectPathsRequiringChangelog.map( ( project ) => {
const projectPaths = {};
for ( const projectPath of touchedProjectPathsRequiringChangelog ) {
let project = projectPath;
if ( project.includes( 'plugins/' ) ) {
return project.replace( 'plugins/', '' );
project = project.replace( 'plugins/', '' );
} else if ( project.includes( 'packages/js/' ) ) {
return project.replace( 'packages/js/', '@woocommerce/' );
project = project.replace( 'packages/js/', '@woocommerce/' );
}
return project;
} );
projectPaths[ project ] = projectPath;
}
return projectPaths;
};
/**
@ -175,7 +180,7 @@ export const getAllProjectPaths = async ( tmpRepoPath: string ) => {
* @param {string} fileName changelog file name
* @param {string} baseOwner PR base owner
* @param {string} baseName PR base name
* @return {Array<string>} List of projects that have Jetpack changelogger enabled and have files changed in a PR.
* @return {Object.<string, string>} Paths to projects that have files changed in a PR keyed by the project name.
*/
export const getTouchedProjectsRequiringChangelog = async (
tmpRepoPath: string,