From b6439479a4511bde8c16f186a922cfb76610d373 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Thu, 25 Jun 2020 14:42:00 +0200 Subject: [PATCH] Commit and note groups should be sorted properly. --- .../release-tools/utils/getwriteroptions.js | 14 +-- .../utils/transformcommitutils.js | 22 ++++- .../release-tools/utils/getwriteroptions.js | 64 +++++++++++++ .../utils/transformcommitutils.js | 96 +++++++++++++------ 4 files changed, 155 insertions(+), 41 deletions(-) diff --git a/packages/ckeditor5-dev-env/lib/release-tools/utils/getwriteroptions.js b/packages/ckeditor5-dev-env/lib/release-tools/utils/getwriteroptions.js index 0ca807fb6..13a600549 100644 --- a/packages/ckeditor5-dev-env/lib/release-tools/utils/getwriteroptions.js +++ b/packages/ckeditor5-dev-env/lib/release-tools/utils/getwriteroptions.js @@ -8,7 +8,7 @@ const fs = require( 'fs' ); const path = require( 'path' ); const templatePath = path.join( __dirname, '..', 'templates' ); -const { typesOrder } = require( './transformcommitutils' ); +const { getTypeOrder } = require( './transformcommitutils' ); /** * @param {Function|Object} transform @@ -18,16 +18,16 @@ module.exports = function getWriterOptions( transform ) { return { transform, groupBy: 'type', - commitGroupsSort( a, b ) { - return typesOrder[ a.title ] - typesOrder[ b.title ]; - }, + commitGroupsSort: sortFunction, commitsSort: [ 'subject' ], - noteGroupsSort( a, b ) { - return typesOrder[ a.title ] - typesOrder[ b.title ]; - }, + noteGroupsSort: sortFunction, mainTemplate: fs.readFileSync( path.join( templatePath, 'template.hbs' ), 'utf-8' ), headerPartial: fs.readFileSync( path.join( templatePath, 'header.hbs' ), 'utf-8' ), commitPartial: fs.readFileSync( path.join( templatePath, 'commit.hbs' ), 'utf-8' ), footerPartial: fs.readFileSync( path.join( templatePath, 'footer.hbs' ), 'utf-8' ) }; }; + +function sortFunction( a, b ) { + return getTypeOrder( a.title ) - getTypeOrder( b.title ); +} diff --git a/packages/ckeditor5-dev-env/lib/release-tools/utils/transformcommitutils.js b/packages/ckeditor5-dev-env/lib/release-tools/utils/transformcommitutils.js index e72cee4b5..84dab6012 100644 --- a/packages/ckeditor5-dev-env/lib/release-tools/utils/transformcommitutils.js +++ b/packages/ckeditor5-dev-env/lib/release-tools/utils/transformcommitutils.js @@ -7,7 +7,7 @@ const getPackageJson = require( './getpackagejson' ); -const transformcommitutils = { +const transformCommitUtils = { /** * A regexp for extracting additional changelog entries from the single commit. * Prefixes of the commit must be synchronized the `getCommitType()` util. @@ -43,6 +43,22 @@ const transformcommitutils = { 'BREAKING CHANGES': 3 }, + /** + * Returns an order of a message in the changelog. + * + * @param {String} title + * @returns {Number} + */ + getTypeOrder( title ) { + for ( const typeTitle of Object.keys( transformCommitUtils.typesOrder ) ) { + if ( title.startsWith( typeTitle ) ) { + return transformCommitUtils.typesOrder[ typeTitle ]; + } + } + + return 10; + }, + /** * Replaces reference to the user (`@name`) with a link to the user's profile. * @@ -72,7 +88,7 @@ const transformcommitutils = { return `[${ maybeRepository }#${ issueId }](https://github.com/${ maybeRepository }/issues/${ issueId })`; } - const repositoryUrl = transformcommitutils.getRepositoryUrl(); + const repositoryUrl = transformCommitUtils.getRepositoryUrl(); // But if doesn't, let's add it. return `[#${ issueId }](${ repositoryUrl }/issues/${ issueId })`; @@ -144,4 +160,4 @@ const transformcommitutils = { } }; -module.exports = transformcommitutils; +module.exports = transformCommitUtils; diff --git a/packages/ckeditor5-dev-env/tests/release-tools/utils/getwriteroptions.js b/packages/ckeditor5-dev-env/tests/release-tools/utils/getwriteroptions.js index 312b166ed..dc5831e63 100644 --- a/packages/ckeditor5-dev-env/tests/release-tools/utils/getwriteroptions.js +++ b/packages/ckeditor5-dev-env/tests/release-tools/utils/getwriteroptions.js @@ -39,5 +39,69 @@ describe( 'dev-env/release-tools/utils', () => { expect( writerOptions.commitGroupsSort ).to.be.a( 'function' ); expect( writerOptions.noteGroupsSort ).to.be.a( 'function' ); } ); + + it( 'sorts notes properly', () => { + const writerOptions = getWriterOptions( transformSpy ); + + const noteGroups = [ + { title: 'BREAKING CHANGES', notes: [] }, + { title: 'MINOR BREAKING CHANGES', notes: [] }, + { title: 'MAJOR BREAKING CHANGES', notes: [] } + ]; + + expect( noteGroups.sort( writerOptions.noteGroupsSort ) ).to.deep.equal( [ + { title: 'MAJOR BREAKING CHANGES', notes: [] }, + { title: 'MINOR BREAKING CHANGES', notes: [] }, + { title: 'BREAKING CHANGES', notes: [] } + ] ); + } ); + + it( 'sorts notes properly (titles with emojis)', () => { + const writerOptions = getWriterOptions( transformSpy ); + + const noteGroups = [ + { title: 'BREAKING CHANGES [ℹ](url)', notes: [] }, + { title: 'MINOR BREAKING CHANGES [ℹ](url)', notes: [] }, + { title: 'MAJOR BREAKING CHANGES [ℹ](url)', notes: [] } + ]; + + expect( noteGroups.sort( writerOptions.noteGroupsSort ) ).to.deep.equal( [ + { title: 'MAJOR BREAKING CHANGES [ℹ](url)', notes: [] }, + { title: 'MINOR BREAKING CHANGES [ℹ](url)', notes: [] }, + { title: 'BREAKING CHANGES [ℹ](url)', notes: [] } + ] ); + } ); + + it( 'sorts groups properly', () => { + const writerOptions = getWriterOptions( transformSpy ); + + const commitGroups = [ + { title: 'Other changes', commits: [] }, + { title: 'Features', commits: [] }, + { title: 'Bug fixes', commits: [] } + ]; + + expect( commitGroups.sort( writerOptions.commitGroupsSort ) ).to.deep.equal( [ + { title: 'Features', commits: [] }, + { title: 'Bug fixes', commits: [] }, + { title: 'Other changes', commits: [] } + ] ); + } ); + + it( 'sorts groups properly (titles with emojis)', () => { + const writerOptions = getWriterOptions( transformSpy ); + + const commitGroups = [ + { title: 'Other changes [ℹ](url)', commits: [] }, + { title: 'Features [ℹ](url)', commits: [] }, + { title: 'Bug fixes [ℹ](url)', commits: [] } + ]; + + expect( commitGroups.sort( writerOptions.commitGroupsSort ) ).to.deep.equal( [ + { title: 'Features [ℹ](url)', commits: [] }, + { title: 'Bug fixes [ℹ](url)', commits: [] }, + { title: 'Other changes [ℹ](url)', commits: [] } + ] ); + } ); } ); } ); diff --git a/packages/ckeditor5-dev-env/tests/release-tools/utils/transformcommitutils.js b/packages/ckeditor5-dev-env/tests/release-tools/utils/transformcommitutils.js index 0fefe7ba9..30ebcd596 100644 --- a/packages/ckeditor5-dev-env/tests/release-tools/utils/transformcommitutils.js +++ b/packages/ckeditor5-dev-env/tests/release-tools/utils/transformcommitutils.js @@ -10,7 +10,7 @@ const sinon = require( 'sinon' ); const proxyquire = require( 'proxyquire' ); describe( 'dev-env/release-tools/utils', () => { - let transformCommit, sandbox, stubs; + let transformCommitUtils, sandbox, stubs; describe( 'transformCommitUtils', () => { beforeEach( () => { @@ -20,7 +20,7 @@ describe( 'dev-env/release-tools/utils', () => { getPackageJson: sandbox.stub() }; - transformCommit = proxyquire( '../../../lib/release-tools/utils/transformcommitutils', { + transformCommitUtils = proxyquire( '../../../lib/release-tools/utils/transformcommitutils', { './getpackagejson': stubs.getPackageJson } ); } ); @@ -31,60 +31,94 @@ describe( 'dev-env/release-tools/utils', () => { describe( 'availableTypes', () => { it( 'should be defined', () => { - expect( transformCommit.availableCommitTypes ).to.be.a( 'Map' ); + expect( transformCommitUtils.availableCommitTypes ).to.be.a( 'Map' ); } ); } ); describe( 'typesOrder', () => { it( 'should be defined', () => { - expect( transformCommit.typesOrder ).to.be.a( 'Object' ); + expect( transformCommitUtils.typesOrder ).to.be.a( 'Object' ); } ); } ); describe( 'MULTI_ENTRIES_COMMIT_REGEXP', () => { it( 'should be defined', () => { - expect( transformCommit.MULTI_ENTRIES_COMMIT_REGEXP ).to.be.a( 'RegExp' ); + expect( transformCommitUtils.MULTI_ENTRIES_COMMIT_REGEXP ).to.be.a( 'RegExp' ); + } ); + } ); + + describe( 'getTypeOrder()', () => { + it( 'returns proper values for commit groups', () => { + expect( transformCommitUtils.getTypeOrder( 'Features' ) ).to.equal( 1 ); + expect( transformCommitUtils.getTypeOrder( 'Bug fixes' ) ).to.equal( 2 ); + expect( transformCommitUtils.getTypeOrder( 'Other changes' ) ).to.equal( 3 ); + } ); + + it( 'returns proper values for commit groups when they ends with additional link', () => { + expect( transformCommitUtils.getTypeOrder( 'Features [ℹ](url)' ) ).to.equal( 1 ); + expect( transformCommitUtils.getTypeOrder( 'Bug fixes [ℹ](url)' ) ).to.equal( 2 ); + expect( transformCommitUtils.getTypeOrder( 'Other changes [ℹ](url)' ) ).to.equal( 3 ); + } ); + + it( 'returns proper values for note groups', () => { + expect( transformCommitUtils.getTypeOrder( 'MAJOR BREAKING CHANGES' ) ).to.equal( 1 ); + expect( transformCommitUtils.getTypeOrder( 'MINOR BREAKING CHANGES' ) ).to.equal( 2 ); + expect( transformCommitUtils.getTypeOrder( 'BREAKING CHANGES' ) ).to.equal( 3 ); + } ); + + it( 'returns proper values for note groups when they ends with additional link', () => { + expect( transformCommitUtils.getTypeOrder( 'MAJOR BREAKING CHANGES [ℹ](url)' ) ).to.equal( 1 ); + expect( transformCommitUtils.getTypeOrder( 'MINOR BREAKING CHANGES [ℹ](url)' ) ).to.equal( 2 ); + expect( transformCommitUtils.getTypeOrder( 'BREAKING CHANGES [ℹ](url)' ) ).to.equal( 3 ); + } ); + + it( 'returns default value for unknown type', () => { + expect( transformCommitUtils.getTypeOrder( 'Foo' ) ).to.equal( 10 ); + expect( transformCommitUtils.getTypeOrder( 'Bar' ) ).to.equal( 10 ); + + expect( transformCommitUtils.getTypeOrder( 'Foo ℹ' ) ).to.equal( 10 ); + expect( transformCommitUtils.getTypeOrder( 'Bar ℹ' ) ).to.equal( 10 ); } ); } ); describe( 'linkToGithubUser()', () => { it( 'makes a link to GitHub profile if a user was mentioned in a comment', () => { - expect( transformCommit.linkToGithubUser( 'Foo @CKSource Bar' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Foo @CKSource Bar' ) ) .to.equal( 'Foo [@CKSource](https://github.com/CKSource) Bar' ); } ); it( 'makes a link to GitHub profile if a user was mentioned in a comment at the beginning', () => { - expect( transformCommit.linkToGithubUser( '@CKSource Bar' ) ) + expect( transformCommitUtils.linkToGithubUser( '@CKSource Bar' ) ) .to.equal( '[@CKSource](https://github.com/CKSource) Bar' ); } ); it( 'makes a link to GitHub profile if a user was mentioned at the beginning of a line', () => { - expect( transformCommit.linkToGithubUser( 'Foo\n@CKSource Bar' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Foo\n@CKSource Bar' ) ) .to.equal( 'Foo\n[@CKSource](https://github.com/CKSource) Bar' ); } ); it( 'makes a link to GitHub profile if a user was mentioned in a comment at the ending', () => { - expect( transformCommit.linkToGithubUser( 'Bar @CKSource' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Bar @CKSource' ) ) .to.equal( 'Bar [@CKSource](https://github.com/CKSource)' ); } ); it( 'makes a link to GitHub profile if a user was mentioned in a bracket', () => { - expect( transformCommit.linkToGithubUser( 'Bar (@CKSource)' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Bar (@CKSource)' ) ) .to.equal( 'Bar ([@CKSource](https://github.com/CKSource))' ); } ); it( 'does nothing if a comment contains scoped package name', () => { - expect( transformCommit.linkToGithubUser( 'Foo @ckeditor/ckeditor5-foo Bar' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Foo @ckeditor/ckeditor5-foo Bar' ) ) .to.equal( 'Foo @ckeditor/ckeditor5-foo Bar' ); } ); it( 'does nothing if an email is inside the comment', () => { - expect( transformCommit.linkToGithubUser( 'Foo foo@bar.com Bar' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Foo foo@bar.com Bar' ) ) .to.equal( 'Foo foo@bar.com Bar' ); } ); it( 'does nothing if a user is already linked', () => { - expect( transformCommit.linkToGithubUser( 'Foo [@bar](https://github.com/bar) Bar' ) ) + expect( transformCommitUtils.linkToGithubUser( 'Foo [@bar](https://github.com/bar) Bar' ) ) .to.equal( 'Foo [@bar](https://github.com/bar) Bar' ); } ); } ); @@ -96,41 +130,41 @@ describe( 'dev-env/release-tools/utils', () => { repository: 'https://github.com/ckeditor/ckeditor5-dev' } ); - expect( transformCommit.linkToGithubIssue( 'Some issue #1.' ) ) + expect( transformCommitUtils.linkToGithubIssue( 'Some issue #1.' ) ) .to.equal( 'Some issue [#1](https://github.com/ckeditor/ckeditor5-dev/issues/1).' ); } ); it( 'replaces "organization/repository#id" with a link to the issue in specified repository', () => { - expect( transformCommit.linkToGithubIssue( 'ckeditor/ckeditor5-dev#1' ) ) + expect( transformCommitUtils.linkToGithubIssue( 'ckeditor/ckeditor5-dev#1' ) ) .to.equal( '[ckeditor/ckeditor5-dev#1](https://github.com/ckeditor/ckeditor5-dev/issues/1)' ); } ); it( 'does not make a link from a comment which is a path', () => { - expect( transformCommit.linkToGithubIssue( 'i/am/a/path#1' ) ) + expect( transformCommitUtils.linkToGithubIssue( 'i/am/a/path#1' ) ) .to.equal( 'i/am/a/path#1' ); } ); it( 'does not make a link if a comment does not match to "organization/repository"', () => { - expect( transformCommit.linkToGithubIssue( 'ckeditor/ckeditor5-dev/' ) ) + expect( transformCommitUtils.linkToGithubIssue( 'ckeditor/ckeditor5-dev/' ) ) .to.equal( 'ckeditor/ckeditor5-dev/' ); } ); it( 'does not make a link from a comment which does not contain the issue id', () => { - expect( transformCommit.linkToGithubIssue( 'ckeditor/ckeditor5-dev#' ) ) + expect( transformCommitUtils.linkToGithubIssue( 'ckeditor/ckeditor5-dev#' ) ) .to.equal( 'ckeditor/ckeditor5-dev#' ); } ); } ); describe( 'getCommitType()', () => { it( 'throws an error when passed unsupported commit type', () => { - expect( () => transformCommit.getCommitType( 'invalid' ) ) + expect( () => transformCommitUtils.getCommitType( 'invalid' ) ) .to.throw( Error, 'Given invalid type of commit ("invalid").' ); } ); it( 'changes a singular type of commit to plural', () => { - expect( transformCommit.getCommitType( 'Feature' ) ).to.equal( 'Features' ); - expect( transformCommit.getCommitType( 'Fix' ) ).to.equal( 'Bug fixes' ); - expect( transformCommit.getCommitType( 'Other' ) ).to.equal( 'Other changes' ); + expect( transformCommitUtils.getCommitType( 'Feature' ) ).to.equal( 'Features' ); + expect( transformCommitUtils.getCommitType( 'Fix' ) ).to.equal( 'Bug fixes' ); + expect( transformCommitUtils.getCommitType( 'Other' ) ).to.equal( 'Other changes' ); } ); } ); @@ -138,13 +172,13 @@ describe( 'dev-env/release-tools/utils', () => { it( 'does not modify too short sentence', () => { const sentence = 'This is a short sentence.'; - expect( transformCommit.truncate( sentence, 25 ) ).to.equal( sentence ); + expect( transformCommitUtils.truncate( sentence, 25 ) ).to.equal( sentence ); } ); it( 'truncates too long sentence', () => { const sentence = 'This is a short sentence.'; - expect( transformCommit.truncate( sentence, 13 ) ).to.equal( 'This is a...' ); + expect( transformCommitUtils.truncate( sentence, 13 ) ).to.equal( 'This is a...' ); } ); } ); @@ -154,7 +188,7 @@ describe( 'dev-env/release-tools/utils', () => { name: 'test-package' } ); - expect( () => transformCommit.getRepositoryUrl() ) + expect( () => transformCommitUtils.getRepositoryUrl() ) .to.throw( Error, 'The package.json for "test-package" must contain the "repository" property.' ); } ); @@ -164,7 +198,7 @@ describe( 'dev-env/release-tools/utils', () => { repository: 'https://github.com/ckeditor/ckeditor5-dev/issues' } ); - transformCommit.getRepositoryUrl( 'foo' ); + transformCommitUtils.getRepositoryUrl( 'foo' ); expect( stubs.getPackageJson.calledOnce ).to.equal( true ); expect( stubs.getPackageJson.firstCall.args[ 0 ] ).to.equal( 'foo' ); @@ -176,7 +210,7 @@ describe( 'dev-env/release-tools/utils', () => { repository: 'https://github.com/ckeditor/ckeditor5-dev/issues' } ); - expect( transformCommit.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); + expect( transformCommitUtils.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); } ); it( 'returns the repository URL (packageJson.repository as a string, ends with ".git")', () => { @@ -185,7 +219,7 @@ describe( 'dev-env/release-tools/utils', () => { repository: 'https://github.com/ckeditor/ckeditor5-dev.git' } ); - expect( transformCommit.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); + expect( transformCommitUtils.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); } ); it( 'returns the repository URL (packageJson.repository as an object)', () => { @@ -196,7 +230,7 @@ describe( 'dev-env/release-tools/utils', () => { } } ); - expect( transformCommit.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); + expect( transformCommitUtils.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); } ); it( 'returns the repository URL (packageJson.repository as an object, contains "/issues")', () => { @@ -208,7 +242,7 @@ describe( 'dev-env/release-tools/utils', () => { } } ); - expect( transformCommit.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); + expect( transformCommitUtils.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); } ); it( 'returns the repository URL (packageJson.repository as an object, ends with ".git")', () => { @@ -220,7 +254,7 @@ describe( 'dev-env/release-tools/utils', () => { } } ); - expect( transformCommit.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); + expect( transformCommitUtils.getRepositoryUrl() ).to.equal( 'https://github.com/ckeditor/ckeditor5-dev' ); } ); } ); } );