Skip to content

Commit

Permalink
Automate grouping of release Changelog PRs by feature (#33229)
Browse files Browse the repository at this point in the history 
* 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 <getdavemail@gmail.com>

* Add feature mapping as provided by Hector

See #33229 (review)

* 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>
  • Loading branch information
@getdave @priethor
getdave and priethor authored Jul 20, 2021
1 parent 94ab1a6 commit d92e2ac
Show file tree
Hide file tree
Showing 2 changed files with 288 additions and 2 deletions.
201 changes: 199 additions & 2 deletions bin/plugin/commands/changelog.js
View file
Original file line number Diff line number Diff line change
@@ -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' );
Expand All @@ -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 */
Expand Down Expand Up @@ -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<string,string>}
*/
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.
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -180,6 +256,7 @@ function getTypesByTitle( title ) {
*/
function getIssueType( issue ) {
const labels = issue.labels.map( ( { name } ) => name );

const candidates = [
...getTypesByLabels( labels ),
...getTypesByTitle( issue.title ),
Expand All @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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 <ul> for the Feature group.
changelog += '- ' + featureName + '\n';

// Add a <li> 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 <ul> 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.<string, IssuesListForRepoResponseItem[]>} 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.
*
Expand Down Expand Up @@ -562,6 +758,7 @@ async function getReleaseChangelog( options ) {
getNormalizedTitle,
getReleaseChangelog,
getIssueType,
getIssueFeature,
sortGroup,
getTypesByLabels,
getTypesByTitle,
Expand Down
89 changes: 89 additions & 0 deletions bin/plugin/commands/test/changelog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
sortGroup,
getTypesByLabels,
getTypesByTitle,
getIssueFeature,
} from '../changelog';

describe( 'getNormalizedTitle', () => {
Expand Down Expand Up @@ -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 = [
Expand Down

0 comments on commit d92e2ac

Please sign in to comment.