From d92e2ac04e6c1d5f2d1215cded3639ac3abd9b7d Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 20 Jul 2021 10:53:26 +0100 Subject: [PATCH] Automate grouping of release `Changelog` PRs by feature (#33229) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial pass at grouping by feature * Basic sort by feature implementation * Prefer Group by feature label if present * Sort Unknown to be the last feature group * Strip feature name from entry * Improve function naming and simplify * Update "Widgets" changelog mapping name to "Widgets Editor" Co-authored-by: Dave Smith * Add feature mapping as provided by Hector See https://github.com/WordPress/gutenberg/pull/33229#pullrequestreview-701879448 * Give priority to explicit label to feature mapping and update tests * Sort features within section by total number of PR entries * Simplify and unify sorting mechanic * Improve code formatting and update UNKNOWN type * Normalize function name * Fix typings Co-authored-by: Héctor <27339341+priethor@users.noreply.github.com> --- bin/plugin/commands/changelog.js | 201 +++++++++++++++++++++++++- bin/plugin/commands/test/changelog.js | 89 ++++++++++++ 2 files changed, 288 insertions(+), 2 deletions(-) diff --git a/bin/plugin/commands/changelog.js b/bin/plugin/commands/changelog.js index 9384bc6540f439..0e769daab44f1d 100644 --- a/bin/plugin/commands/changelog.js +++ b/bin/plugin/commands/changelog.js @@ -1,7 +1,7 @@ /** * External dependencies */ -const { groupBy, escapeRegExp, uniq } = require( 'lodash' ); +const { countBy, groupBy, escapeRegExp, uniq } = require( 'lodash' ); const Octokit = require( '@octokit/rest' ); const { sprintf } = require( 'sprintf-js' ); const semver = require( 'semver' ); @@ -19,6 +19,8 @@ const config = require( '../config' ); // @ts-ignore const manifest = require( '../../../package.json' ); +const UNKNOWN_FEATURE_FALLBACK_NAME = 'Uncategorized'; + /** @typedef {import('@octokit/rest')} GitHub */ /** @typedef {import('@octokit/rest').IssuesListForRepoResponseItem} IssuesListForRepoResponseItem */ /** @typedef {import('@octokit/rest').IssuesListMilestonesForRepoResponseItem} OktokitIssuesListMilestonesForRepoResponseItem */ @@ -92,6 +94,41 @@ const LABEL_TYPE_MAPPING = { '[Type] Security': 'Security', }; +/** + * Mapping of label names to arbitary features in the release notes. + * + * Mapping a given label to a feature will guarantee it will be categorised + * under that feature name in the changelog within each section. + * + * @type {Record} + */ +const LABEL_FEATURE_MAPPING = { + '[Feature] Widgets Screen': 'Widgets Editor', + '[Feature] Design Tools': 'Design Tools', + '[Feature] UI Components': 'Components', + '[Feature] Component System': 'Components', + '[Feature] Template Editing Mode': 'Template Editor', + '[Feature] Writing Flow': 'Block Editor', + '[Feature] Blocks': 'Block Library', + '[Feature] Inserter': 'Block Editor', + '[Feature] Drag and Drop': 'Block Editor', + '[Feature] Block Multi Selection': 'Block Editor', + '[Feature] Link Editing': 'Block Editor', + '[Package] Edit Post': 'Post Editor', + '[Package] Icons': 'Icons', + '[Package] Block Editor': 'Block Editor', + '[Package] Editor': 'Editor', + '[Package] Edit Widgets': 'Widgets Editor', + '[Package] Widgets Customizer': 'Widgets Editor', + '[Package] Components': 'Components', + '[Package] Block Library': 'Block Library', + '[Package] Rich text': 'Block Editor', + '[Package] Data': 'Data Layer', + '[Block] Legacy Widget': 'Widgets Editor', + 'REST API Interaction': 'REST API', + 'New Block': 'Block Library', +}; + /** * Order in which to print group titles. A value of `undefined` is used as slot * in which unrecognized headings are to be inserted. @@ -152,6 +189,45 @@ function getTypesByLabels( labels ) { ); } +/** + * Returns candidates by retrieving the appropriate mapping + * from the label -> feature lookup. + * + * @param {string[]} labels Label names. + * + * @return {string[]} Feature candidates. + */ +function mapLabelsToFeatures( labels ) { + return labels + .filter( ( label ) => + Object.keys( LABEL_FEATURE_MAPPING ).includes( label ) + ) + .map( ( label ) => LABEL_FEATURE_MAPPING[ label ] ); +} + +/** + * Returns whether not the given labels contain the block specific + * label "block library". + * + * @param {string[]} labels Label names. + * + * @return {boolean} whether or not the issue's is labbeled as block specific + */ +function getIsBlockSpecificIssue( labels ) { + return !! labels.find( ( label ) => label.startsWith( '[Block] ' ) ); +} + +/** + * Returns the first feature specific label from the given labels. + * + * @param {string[]} labels Label names. + * + * @return {string|undefined} the feature specific label. + */ +function getFeatureSpecificLabels( labels ) { + return labels.find( ( label ) => label.startsWith( '[Feature] ' ) ); +} + /** * Returns type candidates based on given issue title. * @@ -180,6 +256,7 @@ function getTypesByTitle( title ) { */ function getIssueType( issue ) { const labels = issue.labels.map( ( { name } ) => name ); + const candidates = [ ...getTypesByLabels( labels ), ...getTypesByTitle( issue.title ), @@ -188,6 +265,51 @@ function getIssueType( issue ) { return candidates.length ? candidates.sort( sortType )[ 0 ] : 'Various'; } +/** + * Returns the most appropriate feature category for the given issue based + * on a basic heuristic. + * + * @param {IssuesListForRepoResponseItem} issue Issue object. + * + * @return {string} the feature name. + */ +function getIssueFeature( issue ) { + const labels = issue.labels.map( ( { name } ) => name ); + + const featureCandidates = mapLabelsToFeatures( labels ); + + // 1. Prefer explicit mapping of label to feature. + if ( featureCandidates.length ) { + // Get occurances of the feature labels. + const featureCounts = countBy( featureCandidates ); + + // Check which matching label occurs most often. + const rankedFeatures = Object.keys( featureCounts ).sort( + ( a, b ) => featureCounts[ b ] - featureCounts[ a ] + ); + + // Return the one that appeared most often. + return rankedFeatures[ 0 ]; + } + + // 2. `[Feature]` labels + const featureSpecificLabel = getFeatureSpecificLabels( labels ); + + if ( featureSpecificLabel ) { + return removeFeaturePrefix( featureSpecificLabel ); + } + + // 3. Block specific labels. + const blockSpecificLabels = getIsBlockSpecificIssue( labels ); + + if ( blockSpecificLabels ) { + return 'Block Library'; + } + + // Fallback - if we couldn't find a good match. + return UNKNOWN_FEATURE_FALLBACK_NAME; +} + /** * Sort comparator, comparing two label candidates. * @@ -325,6 +447,17 @@ function removeRedundantTypePrefix( title, issue ) { ); } +/** + * Removes any `[Feature] ` prefix from a given string. + * + * @param {string} text The string of text potentially containing a prefix. + * + * @return {string} the text without the prefix. + */ +function removeFeaturePrefix( text ) { + return text.replace( '[Feature] ', '' ); +} + /** * Array of normalizations applying to title, each returning a new string, or * undefined to indicate an entry which should be omitted. @@ -485,7 +618,9 @@ async function getChangelog( settings ) { let changelog = ''; const groupedPullRequests = groupBy( pullRequests, getIssueType ); + const sortedGroups = Object.keys( groupedPullRequests ).sort( sortGroup ); + for ( const group of sortedGroups ) { const groupPullRequests = groupedPullRequests[ group ]; const groupEntries = groupPullRequests @@ -496,14 +631,75 @@ async function getChangelog( settings ) { continue; } + // Start a new section within the changelog. changelog += '### ' + group + '\n\n'; - groupEntries.forEach( ( entry ) => ( changelog += entry + '\n' ) ); + + // Group PRs within this section into "Features". + const featureGroups = groupBy( groupPullRequests, getIssueFeature ); + + const featuredGroupNames = sortFeatureGroups( featureGroups ); + + // Start output of Features within the section. + featuredGroupNames.forEach( ( featureName ) => { + const featureGroupPRs = featureGroups[ featureName ]; + + const featureGroupEntries = featureGroupPRs + .map( getEntry ) + .filter( Boolean ); + + // Don't create feature sections when there are no PRs. + if ( ! featureGroupEntries.length ) { + return; + } + + // Start new
    for the Feature group. + changelog += '- ' + featureName + '\n'; + + // Add a
  • for each PR in the Feature. + featureGroupEntries.forEach( ( entry ) => { + // Strip feature name from entry if present. + entry = entry && entry.replace( `[${ featureName } - `, '[' ); + + // Add a new bullet point to the list. + changelog += ` ${ entry }\n`; + } ); + + // Close the
      for the Feature group. + changelog += '\n'; + } ); + changelog += '\n'; } return changelog; } +/** + * Sorts the feature groups by the feature which contains the greatest number of PRs + * ready for output into the changelog. + * + * @param {Object.} featureGroups feature specific PRs keyed by feature name. + * @return {string[]} sorted list of feature names. + */ +function sortFeatureGroups( featureGroups ) { + return Object.keys( featureGroups ).sort( + ( featureAName, featureBName ) => { + // Sort "Unknown" to always be at the end + if ( featureAName === UNKNOWN_FEATURE_FALLBACK_NAME ) { + return 1; + } else if ( featureBName === UNKNOWN_FEATURE_FALLBACK_NAME ) { + return -1; + } + + // Sort by greatest number of PRs in the group first. + return ( + featureGroups[ featureBName ].length - + featureGroups[ featureAName ].length + ); + } + ); +} + /** * Generates and logs changelog for a milestone. * @@ -562,6 +758,7 @@ async function getReleaseChangelog( options ) { getNormalizedTitle, getReleaseChangelog, getIssueType, + getIssueFeature, sortGroup, getTypesByLabels, getTypesByTitle, diff --git a/bin/plugin/commands/test/changelog.js b/bin/plugin/commands/test/changelog.js index bdde2e92926c9f..773609949873b3 100644 --- a/bin/plugin/commands/test/changelog.js +++ b/bin/plugin/commands/test/changelog.js @@ -12,6 +12,7 @@ import { sortGroup, getTypesByLabels, getTypesByTitle, + getIssueFeature, } from '../changelog'; describe( 'getNormalizedTitle', () => { @@ -172,6 +173,94 @@ describe( 'getIssueType', () => { } ); } ); +describe( 'getIssueFeature', () => { + it( 'returns "Unknown" as feature if there are no labels', () => { + const result = getIssueFeature( { labels: [] } ); + + expect( result ).toBe( 'Uncategorized' ); + } ); + + it( 'falls by to "Unknown" as the feature if unable to classify by other means', () => { + const result = getIssueFeature( { + labels: [ + { + name: 'Some Label', + }, + { + name: '[Package] Example Package', // 1. has explicit mapping + }, + { + name: '[Package] Another One', + }, + ], + } ); + + expect( result ).toEqual( 'Uncategorized' ); + } ); + + it( 'gives precedence to manual feature mapping', () => { + const result = getIssueFeature( { + labels: [ + { + name: '[Block] Some Block', // 3. Block-specific label + }, + { + name: '[Package] Edit Widgets', // 1. has explicit mapping + }, + { + name: '[Feature] Some Feature', // 2. Feature label. + }, + { + name: '[Package] Another One', + }, + ], + } ); + + const mappingForPackageEditWidgets = 'Widgets Editor'; + + expect( result ).toEqual( mappingForPackageEditWidgets ); + } ); + + it( 'gives secondary priority to feature labels when manually mapped label is not present', () => { + const result = getIssueFeature( { + labels: [ + { + name: '[Block] Some Block', // block specific label + }, + { + name: '[Package] This package', + }, + { + name: '[Feature] Cool Feature', // should have priority despite prescence of block specific label + }, + { + name: '[Package] Another One', + }, + ], + } ); + + expect( result ).toEqual( 'Cool Feature' ); + } ); + + it( 'gives tertiary priority to "Block Library" as feature for all PRs that have a block specific label (and where manually mapped or feature label not present)', () => { + const result = getIssueFeature( { + labels: [ + { + name: '[Block] Some Block', + }, + { + name: '[Package] This package', + }, + { + name: '[Package] Another One', + }, + ], + } ); + + expect( result ).toEqual( 'Block Library' ); + } ); +} ); + describe( 'sortGroup', () => { it( 'returns groups in order', () => { const result = [