Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gerrit): use commit message footers to store source branch name #29802

Merged
merged 19 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
eeaf981
feat(gerrit): use commit message footers instead of hashtags
felipecrs Jun 21, 2024
8bf41bc
chore(gerrit): avoid an empty line between footers
felipecrs Jun 21, 2024
8465fbd
chore(gerrit): fix tests after lastavoiding empty line between footers
felipecrs Jun 21, 2024
0e4de2f
chore(gerrit): fix lint error after last commit
felipecrs Jun 21, 2024
2b7a3da
chore(gerrit): delete spurious orig files
felipecrs Jun 21, 2024
252b288
chore(gerrit): restore backwards compatibility with hashtags
felipecrs Jun 24, 2024
69a01b1
chore(gerrit): rename Renovate-Source-Branch to Renovate-Branch
felipecrs Jun 24, 2024
bf20b2d
chore(gerrit): update documentation
felipecrs Jun 24, 2024
9fa72ce
Merge branch 'main' of https://github.com/renovatebot/renovate into u…
felipecrs Jun 24, 2024
c4b3934
chore(gerrit): improve some conditionals
felipecrs Jun 24, 2024
ce48e4d
chore(gerrit): avoid multiple array push calls
felipecrs Jun 24, 2024
af0284b
Apply suggestions from code review
felipecrs Jun 26, 2024
a1d714e
Merge branch 'main' into use-footers-gerrit
felipecrs Jun 26, 2024
2503509
Apply suggestions from code review
felipecrs Jun 28, 2024
b3725ca
Merge branch 'main' of https://github.com/renovatebot/renovate into u…
felipecrs Jun 28, 2024
9470849
Fix review changes
felipecrs Jun 28, 2024
a514cd7
Try to fix coverage
felipecrs Jun 28, 2024
e1c8215
Fix Gerrit filter missing braces
felipecrs Jul 2, 2024
19bc4d9
Merge branch 'main' of https://github.com/renovatebot/renovate into u…
felipecrs Jul 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/modules/platform/gerrit/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ describe('modules/platform/gerrit/client', () => {
['owner:self', { branchName: 'dependency-xyz' }],
['project:repo', { branchName: 'dependency-xyz' }],
['-is:wip', { branchName: 'dependency-xyz' }],
['hashtag:sourceBranch-dependency-xyz', { branchName: 'dependency-xyz' }],
[
'footer:Renovate-Source-Branch=dependency-xyz',
{ branchName: 'dependency-xyz' },
],
['label:Code-Review=-2', { branchName: 'dependency-xyz', label: '-2' }],
[
'branch:otherTarget',
Expand Down
2 changes: 1 addition & 1 deletion lib/modules/platform/gerrit/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class GerritClient {
const filterState = mapPrStateToGerritFilter(searchConfig.state);
const filters = ['owner:self', 'project:' + repository, filterState];
if (searchConfig.branchName !== '') {
filters.push(`hashtag:sourceBranch-${searchConfig.branchName}`);
filters.push(`footer:Renovate-Source-Branch=${searchConfig.branchName}`);
}
if (searchConfig.targetBranch) {
filters.push(`branch:${searchConfig.targetBranch}`);
Expand Down
23 changes: 18 additions & 5 deletions lib/modules/platform/gerrit/scm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,20 @@ describe('modules/platform/gerrit/scm', () => {
baseBranch: 'main',
branchName: 'renovate/dependency-1.x',
files: [],
message: ['commit msg', expect.stringMatching(/Change-Id: I.{32}/)],
message: [
'commit msg',
expect.stringMatching(
new RegExp(
'^Renovate-Source-Branch: renovate/dependency-1\\.x\\nChange-Id: I[a-z0-9]{40}$',
),
),
],
force: true,
});
expect(git.pushCommit).toHaveBeenCalledWith({
files: [],
sourceRef: 'renovate/dependency-1.x',
targetRef: 'refs/for/main%t=sourceBranch-renovate/dependency-1.x',
targetRef: 'refs/for/main',
});
});

Expand Down Expand Up @@ -339,7 +346,10 @@ describe('modules/platform/gerrit/scm', () => {
baseBranch: 'main',
branchName: 'renovate/dependency-1.x',
files: [],
message: ['commit msg', 'Change-Id: ...'],
message: [
'commit msg',
'Renovate-Source-Branch: renovate/dependency-1.x\nChange-Id: ...',
],
force: true,
});
expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
Expand Down Expand Up @@ -377,14 +387,17 @@ describe('modules/platform/gerrit/scm', () => {
baseBranch: 'main',
branchName: 'renovate/dependency-1.x',
files: [],
message: ['commit msg', 'Change-Id: ...'],
message: [
'commit msg',
'Renovate-Source-Branch: renovate/dependency-1.x\nChange-Id: ...',
],
force: true,
});
expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
expect(git.pushCommit).toHaveBeenCalledWith({
files: [],
sourceRef: 'renovate/dependency-1.x',
targetRef: 'refs/for/main%t=sourceBranch-renovate/dependency-1.x',
targetRef: 'refs/for/main',
});
expect(clientMock.wasApprovedBy).toHaveBeenCalledWith(
existingChange,
Expand Down
6 changes: 2 additions & 4 deletions lib/modules/platform/gerrit/scm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class GerritScm extends DefaultGitScm {
typeof commit.message === 'string' ? [commit.message] : commit.message;
commit.message = [
...origMsg,
`Change-Id: ${existingChange?.change_id ?? generateChangeId()}`,
`Renovate-Source-Branch: ${commit.branchName}\nChange-Id: ${existingChange?.change_id ?? generateChangeId()}`,
felipecrs marked this conversation as resolved.
Show resolved Hide resolved
];
const commitResult = await git.prepareCommit({ ...commit, force: true });
if (commitResult) {
Expand All @@ -123,9 +123,7 @@ export class GerritScm extends DefaultGitScm {
if (hasChanges || commit.force) {
const pushResult = await git.pushCommit({
sourceRef: commit.branchName,
targetRef: `refs/for/${commit.baseBranch!}%t=sourceBranch-${
commit.branchName
}`,
targetRef: `refs/for/${commit.baseBranch!}`,
files: commit.files,
});
if (pushResult) {
Expand Down
43 changes: 32 additions & 11 deletions lib/modules/platform/gerrit/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
GerritChangeMessageInfo,
GerritChangeStatus,
GerritLabelTypeInfo,
GerritRevisionInfo,
} from './types';
import * as utils from './utils';
import { mapBranchStatusToLabel } from './utils';
Expand Down Expand Up @@ -83,14 +84,22 @@ describe('modules/platform/gerrit/utils', () => {
const change = partial<GerritChange>({
_number: 123456,
status: 'NEW',
hashtags: ['other', 'sourceBranch-renovate/dependency-1.x'],
branch: 'main',
subject: 'Fix for',
reviewers: {
REVIEWER: [partial<GerritAccountInfo>({ username: 'username' })],
REMOVED: [],
CC: [],
},
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message:
'Some change\n\nRenovate-Source-Branch: renovate/dependency-1.x\nChange-Id: ...',
},
}),
},
messages: [
partial<GerritChangeMessageInfo>({
id: '9d78ac236714cee8c2d86e95d638358925cf6853',
Expand Down Expand Up @@ -122,11 +131,10 @@ describe('modules/platform/gerrit/utils', () => {
});
});

it('map a gerrit change without sourceBranch-tag and reviewers to Pr', () => {
it('map a gerrit change without source branch footer and reviewers to Pr', () => {
const change = partial<GerritChange>({
_number: 123456,
status: 'NEW',
hashtags: ['other'],
branch: 'main',
subject: 'Fix for',
});
Expand All @@ -145,23 +153,36 @@ describe('modules/platform/gerrit/utils', () => {
});

describe('extractSourceBranch()', () => {
it('without hashtags', () => {
const change = partial<GerritChange>({
hashtags: undefined,
});
it('no commit message', () => {
const change = partial<GerritChange>();
expect(utils.extractSourceBranch(change)).toBeUndefined();
});

it('no hashtag with "sourceBranch-" prefix', () => {
it('commit message with no footer', () => {
const change = partial<GerritChange>({
hashtags: ['other', 'another'],
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message: 'some message...',
},
}),
},
});
expect(utils.extractSourceBranch(change)).toBeUndefined();
});

it('hashtag with "sourceBranch-" prefix', () => {
it('commit message with footer', () => {
const change = partial<GerritChange>({
hashtags: ['other', 'sourceBranch-renovate/dependency-1.x', 'another'],
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message:
'Some change\n\nRenovate-Source-Branch: renovate/dependency-1.x\nChange-Id: ...',
},
}),
},
});
expect(utils.extractSourceBranch(change)).toBe('renovate/dependency-1.x');
});
Expand Down
11 changes: 8 additions & 3 deletions lib/modules/platform/gerrit/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,14 @@ export function mapGerritChangeStateToPrState(
return 'all';
}
export function extractSourceBranch(change: GerritChange): string | undefined {
return change.hashtags
?.find((tag) => tag.startsWith('sourceBranch-'))
?.replace('sourceBranch-', '');
if (change.current_revision === undefined) {
// This should never happen in production, but it does happen in tests
return undefined;
}

return change.revisions[change.current_revision]?.commit?.message?.match(
/^Renovate-Source-Branch: (.+)$/m,
)?.[1];
}

export function findPullRequestBody(change: GerritChange): string | undefined {
Expand Down