Skip to content

Commit

Permalink
fix: pass pull request header and footer options to merge plugin (#2143)
Browse files Browse the repository at this point in the history 
* fix: pass pull request header and footer options to merge plugin

* chore: add test for header/footer in multi package PRs

* fixup! chore: add test for header/footer in multi package PRs

---------

Co-authored-by: Jeff Ching <chingor@google.com>
  • Loading branch information
@lukekarrys @chingor13
lukekarrys and chingor13 authored Dec 11, 2023
1 parent c84414a commit e848100
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 83 deletions.
2 changes: 1 addition & 1 deletion __snapshots__/manifest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ exports['Manifest buildPullRequests should handle mixing componentless configs 1
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
`

exports['Manifest buildPullRequests should handle multiple package repository 1'] = `
exports['Manifest buildPullRequests with multiple packages should handle multiple package repository 1'] = `
:robot: I have created a release *beep* *boop*
---
Expand Down
31 changes: 27 additions & 4 deletions src/manifest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
} from './factory';
import {Release} from './release';
import {Strategy} from './strategy';
import {Merge} from './plugins/merge';
import {MergeOptions, Merge} from './plugins/merge';
import {ReleasePleaseManifest} from './updaters/release-please-manifest';
import {
DuplicateReleaseError,
Expand Down Expand Up @@ -755,10 +755,33 @@ export class Manifest {
// Combine pull requests into 1 unless configured for separate
// pull requests
if (!this.separatePullRequests) {
const mergeOptions: MergeOptions = {
pullRequestTitlePattern: this.groupPullRequestTitlePattern,
};
// Find the first repositoryConfig item that has a set value
// for the options that can be passed to the merge plugin
for (const path in this.repositoryConfig) {
const config = this.repositoryConfig[path];
if (
'pullRequestHeader' in config &&
!('pullRequestHeader' in mergeOptions)
) {
mergeOptions.pullRequestHeader = config.pullRequestHeader;
}
if (
'pullRequestFooter' in config &&
!('pullRequestFooter' in mergeOptions)
) {
mergeOptions.pullRequestFooter = config.pullRequestFooter;
}
}
this.plugins.push(
new Merge(this.github, this.targetBranch, this.repositoryConfig, {
pullRequestTitlePattern: this.groupPullRequestTitlePattern,
})
new Merge(
this.github,
this.targetBranch,
this.repositoryConfig,
mergeOptions
)
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/merge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {Update} from '../update';
import {mergeUpdates} from '../updaters/composite';
import {GitHub} from '../github';

interface MergeOptions {
export interface MergeOptions {
pullRequestTitlePattern?: string;
pullRequestHeader?: string;
pullRequestFooter?: string;
Expand Down
187 changes: 110 additions & 77 deletions test/manifest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1750,90 +1750,123 @@ describe('Manifest', () => {
);
});

it('should handle multiple package repository', async () => {
mockReleases(sandbox, github, [
{
id: 123456,
sha: 'abc123',
tagName: 'pkg1-v1.0.0',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg1-v1.0.0',
},
{
id: 654321,
sha: 'def234',
tagName: 'pkg2-v0.2.3',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg2-v0.2.3',
},
]);
mockCommits(sandbox, github, [
{
sha: 'aaaaaa',
message: 'fix: some bugfix',
files: ['path/a/foo'],
},
{
sha: 'abc123',
message: 'chore: release main',
files: [],
pullRequest: {
headBranchName: 'release-please/branches/main',
baseBranchName: 'main',
number: 123,
title: 'chore: release main',
body: '',
labels: [],
files: [],
describe('with multiple packages', () => {
beforeEach(() => {
mockReleases(sandbox, github, [
{
id: 123456,
sha: 'abc123',
tagName: 'pkg1-v1.0.0',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg1-v1.0.0',
},
},
{
sha: 'bbbbbb',
message: 'fix: some bugfix',
files: ['path/b/foo'],
},
{
sha: 'cccccc',
message: 'fix: some bugfix',
files: ['path/a/foo'],
},
{
sha: 'def234',
message: 'chore: release main',
files: [],
pullRequest: {
headBranchName: 'release-please/branches/main',
baseBranchName: 'main',
number: 123,
title: 'chore: release main',
body: '',
labels: [],
{
id: 654321,
sha: 'def234',
tagName: 'pkg2-v0.2.3',
url: 'https://github.com/fake-owner/fake-repo/releases/tag/pkg2-v0.2.3',
},
]);
mockCommits(sandbox, github, [
{
sha: 'aaaaaa',
message: 'fix: some bugfix',
files: ['path/a/foo'],
},
{
sha: 'abc123',
message: 'chore: release main',
files: [],
pullRequest: {
headBranchName: 'release-please/branches/main',
baseBranchName: 'main',
number: 123,
title: 'chore: release main',
body: '',
labels: [],
files: [],
sha: 'abc123',
},
},
{
sha: 'bbbbbb',
message: 'fix: some bugfix',
files: ['path/b/foo'],
},
{
sha: 'cccccc',
message: 'fix: some bugfix',
files: ['path/a/foo'],
},
{
sha: 'def234',
message: 'chore: release main',
files: [],
pullRequest: {
headBranchName: 'release-please/branches/main',
baseBranchName: 'main',
number: 123,
title: 'chore: release main',
body: '',
labels: [],
files: [],
sha: 'def234',
},
},
},
]);
const manifest = new Manifest(
github,
'main',
{
'path/a': {
releaseType: 'simple',
component: 'pkg1',
]);
});
it('should handle multiple package repository', async () => {
const manifest = new Manifest(
github,
'main',
{
'path/a': {
releaseType: 'simple',
component: 'pkg1',
},
'path/b': {
releaseType: 'simple',
component: 'pkg2',
},
},
'path/b': {
releaseType: 'simple',
component: 'pkg2',
{
'path/a': Version.parse('1.0.0'),
'path/b': Version.parse('0.2.3'),
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(1);
expect(pullRequests[0].labels).to.eql(['autorelease: pending']);
snapshot(dateSafe(pullRequests[0].body.toString()));
});

it('should handle pull request header/footer with multiple packages', async () => {
const manifest = new Manifest(
github,
'main',
{
'path/a': {
releaseType: 'simple',
component: 'pkg1',
pullRequestHeader: 'Header from pkg1',
},
'path/b': {
releaseType: 'simple',
component: 'pkg2',
pullRequestHeader: 'Header from pkg2',
pullRequestFooter: 'Footer from pkg2',
},
},
},
{
'path/a': Version.parse('1.0.0'),
'path/b': Version.parse('0.2.3'),
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(1);
expect(pullRequests[0].labels).to.eql(['autorelease: pending']);
snapshot(dateSafe(pullRequests[0].body.toString()));
{
'path/a': Version.parse('1.0.0'),
'path/b': Version.parse('0.2.3'),
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(1);
const pullRequest = pullRequests[0];
expect(pullRequest.body.header.toString()).to.eql('Header from pkg1');
expect(pullRequest.body.footer.toString()).to.eql('Footer from pkg2');
});
});

it('should allow creating multiple pull requests', async () => {
Expand Down

0 comments on commit e848100

Please sign in to comment.