Skip to content

Commit

Permalink
feat(release-notes)!: support configurable fetching stage (#22781)
Browse files Browse the repository at this point in the history
Changes fetchReleaseNotes from boolean to enum, with values off, branch, pr.

Closes #20476

BREAKING CHANGE: Release notes won't be fetched early for commitBody insertion unless explicitly configured with fetchReleaseNotes=branch
  • Loading branch information
RahulGautamSingh authored Jun 18, 2023
1 parent 93a7751 commit 6515df8
Show file tree
Hide file tree
Showing 14 changed files with 66 additions and 114 deletions.
9 changes: 8 additions & 1 deletion docs/usage/configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,14 @@ A similar one could strip leading `v` prefixes:

## fetchReleaseNotes

Set this to `false` if you want to disable release notes fetching.
Use this config option to configure release notes fetching.
The available options are:

- `off` - disable release notes fetching
- `branch` - fetch release notes while creating/updating branch
- `pr`(default) - fetches release notes while creating/updating pull-request

It is not recommended to set fetchReleaseNotes=branch unless you are embedding release notes in commit information, because it results in a performance decrease.

Renovate can fetch release notes when they are hosted on one of these platforms:

Expand Down
22 changes: 22 additions & 0 deletions lib/config/migrations/custom/fetch-release-notes-migration.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { FetchReleaseNotesMigration } from './fetch-release-notes-migration';

describe('config/migrations/custom/fetch-release-notes-migration', () => {
it('migrates', () => {
expect(FetchReleaseNotesMigration).toMigrate(
{
fetchReleaseNotes: false as never,
},
{
fetchReleaseNotes: 'off',
}
);
expect(FetchReleaseNotesMigration).toMigrate(
{
fetchReleaseNotes: true as never,
},
{
fetchReleaseNotes: 'pr',
}
);
});
});
12 changes: 12 additions & 0 deletions lib/config/migrations/custom/fetch-release-notes-migration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import is from '@sindresorhus/is';
import { AbstractMigration } from '../base/abstract-migration';

export class FetchReleaseNotesMigration extends AbstractMigration {
override readonly propertyName = 'fetchReleaseNotes';

override run(value: unknown): void {
if (is.boolean(value)) {
this.rewrite(value ? 'pr' : 'off');
}
}
}
2 changes: 2 additions & 0 deletions lib/config/migrations/migrations-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { DepTypesMigration } from './custom/dep-types-migration';
import { DryRunMigration } from './custom/dry-run-migration';
import { EnabledManagersMigration } from './custom/enabled-managers-migration';
import { ExtendsMigration } from './custom/extends-migration';
import { FetchReleaseNotesMigration } from './custom/fetch-release-notes-migration';
import { GoModTidyMigration } from './custom/go-mod-tidy-migration';
import { HostRulesMigration } from './custom/host-rules-migration';
import { IgnoreNodeModulesMigration } from './custom/ignore-node-modules-migration';
Expand Down Expand Up @@ -148,6 +149,7 @@ export class MigrationsService {
DatasourceMigration,
RecreateClosedMigration,
StabilityDaysMigration,
FetchReleaseNotesMigration,
];

static run(originalConfig: RenovateConfig): RenovateConfig {
Expand Down
7 changes: 4 additions & 3 deletions lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2540,9 +2540,10 @@ const options: RenovateOptions[] = [
},
{
name: 'fetchReleaseNotes',
description: 'Controls if release notes are fetched.',
type: 'boolean',
default: true,
description: 'Controls if and when release notes are fetched.',
type: 'string',
allowedValues: ['off', 'branch', 'pr'],
default: 'pr',
cli: false,
env: false,
},
Expand Down
4 changes: 3 additions & 1 deletion lib/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export interface RenovateConfig
vulnerabilitySeverity?: string;
regexManagers?: RegExManager[];

fetchReleaseNotes?: boolean;
fetchReleaseNotes?: FetchReleaseNotesOptions;
secrets?: Record<string, string>;

constraints?: Record<string, string>;
Expand Down Expand Up @@ -299,6 +299,8 @@ export type UpdateType =
| 'bump'
| 'replacement';

export type FetchReleaseNotesOptions = 'off' | 'branch' | 'pr';

export type MatchStringsStrategy = 'any' | 'recursive' | 'combination';

export type MergeStrategy =
Expand Down
21 changes: 1 addition & 20 deletions lib/workers/repository/changelog/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { mockedFunction, partial } from '../../../../test/util';
import type { BranchUpgradeConfig } from '../../types';
import { getChangeLogJSON } from '../update/pr/changelog';
import { embedChangelogs, needsChangelogs } from '.';
import { embedChangelogs } from '.';

jest.mock('../update/pr/changelog');

Expand All @@ -27,23 +27,4 @@ describe('workers/repository/changelog/index', () => {
{ logJSON: null },
]);
});

it('needsChangelogs', () => {
expect(needsChangelogs(partial<BranchUpgradeConfig>())).toBeFalse();
expect(
needsChangelogs(
partial<BranchUpgradeConfig>({
commitBody: '{{#if logJSON.hasReleaseNotes}}has changelog{{/if}}',
})
)
).toBeFalse();
expect(
needsChangelogs(
partial<BranchUpgradeConfig>({
commitBody: '{{#if logJSON.hasReleaseNotes}}has changelog{{/if}}',
}),
['commitBody']
)
).toBeTrue();
});
});
18 changes: 0 additions & 18 deletions lib/workers/repository/changelog/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import * as p from '../../../util/promises';
import {
containsTemplates,
exposedConfigOptions,
} from '../../../util/template';
import type { BranchUpgradeConfig } from '../../types';
import { getChangeLogJSON } from '../update/pr/changelog';

Expand All @@ -21,17 +17,3 @@ export async function embedChangelogs(
): Promise<void> {
await p.map(branches, embedChangelog, { concurrency: 10 });
}

export function needsChangelogs(
upgrade: BranchUpgradeConfig,
fields = exposedConfigOptions.filter((o) => o !== 'commitBody')
): boolean {
// commitBody is now compiled when commit is done
for (const field of fields) {
// fields set by `getChangeLogJSON`
if (containsTemplates(upgrade[field], ['logJSON', 'releases'])) {
return true;
}
}
return false;
}
5 changes: 1 addition & 4 deletions lib/workers/repository/update/branch/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
fs,
git,
mocked,
mockedFunction,
partial,
platform,
scm,
Expand Down Expand Up @@ -34,7 +33,6 @@ import * as _mergeConfidence from '../../../../util/merge-confidence';
import * as _sanitize from '../../../../util/sanitize';
import * as _limits from '../../../global/limits';
import type { BranchConfig, BranchUpgradeConfig } from '../../../types';
import { needsChangelogs } from '../../changelog';
import type { ResultWithPr } from '../pr';
import * as _prWorker from '../pr';
import * as _prAutomerge from '../pr/automerge';
Expand Down Expand Up @@ -816,9 +814,8 @@ describe('workers/repository/update/branch/index', () => {
ignoreTests: true,
prCreation: 'not-pending',
commitBody: '[skip-ci]',
fetchReleaseNotes: true,
fetchReleaseNotes: 'branch',
} satisfies BranchConfig;
mockedFunction(needsChangelogs).mockReturnValueOnce(true);
scm.getBranchCommit.mockResolvedValue('123test'); //TODO:not needed?
expect(await branchWorker.processBranch(inconfig)).toEqual({
branchExists: true,
Expand Down
14 changes: 5 additions & 9 deletions lib/workers/repository/update/branch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { toMs } from '../../../../util/pretty-time';
import * as template from '../../../../util/template';
import { isLimitReached } from '../../../global/limits';
import type { BranchConfig, BranchResult, PrBlockedBy } from '../../../types';
import { embedChangelog, needsChangelogs } from '../../changelog';
import { embedChangelogs } from '../../changelog';
import { ensurePr } from '../pr';
import { checkAutoMerge } from '../pr/automerge';
import { setArtifactErrorStatus } from './artifacts';
Expand Down Expand Up @@ -482,6 +482,10 @@ export async function processBranch(
} else {
logger.debug('No updated lock files in branch');
}
if (config.fetchReleaseNotes === 'branch') {
await embedChangelogs(config.upgrades);
}

const postUpgradeCommandResults = await executePostUpgradeCommands(
config
);
Expand Down Expand Up @@ -540,14 +544,6 @@ export async function processBranch(

// compile commit message with body, which maybe needs changelogs
if (config.commitBody) {
if (
config.fetchReleaseNotes &&
needsChangelogs(config, ['commitBody'])
) {
// we only need first upgrade, the others are only needed on PR update
// we add it to first, so PR fetch can skip fetching for that update
await embedChangelog(config.upgrades[0]);
}
// changelog is on first upgrade
config.commitMessage = `${config.commitMessage!}\n\n${template.compile(
config.commitBody,
Expand Down
10 changes: 5 additions & 5 deletions lib/workers/repository/update/pr/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe('workers/repository/update/pr/index', () => {
platform.createPr.mockResolvedValueOnce(pr);
limits.isLimitReached.mockReturnValueOnce(true);

config.fetchReleaseNotes = true;
config.fetchReleaseNotes = 'pr';

const res = await ensurePr(config);

Expand Down Expand Up @@ -871,13 +871,13 @@ describe('workers/repository/update/pr/index', () => {
bodyFingerprint: fingerprint(
generatePrBodyFingerprintConfig({
...config,
fetchReleaseNotes: true,
fetchReleaseNotes: 'pr',
})
),
lastEdited: new Date('2020-01-20T00:00:00Z').toISOString(),
};
prCache.getPrCache.mockReturnValueOnce(cachedPr);
const res = await ensurePr({ ...config, fetchReleaseNotes: true });
const res = await ensurePr({ ...config, fetchReleaseNotes: 'pr' });
expect(res).toEqual({
type: 'with-pr',
pr: existingPr,
Expand All @@ -904,13 +904,13 @@ describe('workers/repository/update/pr/index', () => {
bodyFingerprint: fingerprint(
generatePrBodyFingerprintConfig({
...config,
fetchReleaseNotes: true,
fetchReleaseNotes: 'pr',
})
),
lastEdited: new Date('2020-01-20T00:00:00Z').toISOString(),
};
prCache.getPrCache.mockReturnValueOnce(cachedPr);
const res = await ensurePr({ ...config, fetchReleaseNotes: true });
const res = await ensurePr({ ...config, fetchReleaseNotes: 'pr' });
expect(res).toEqual({
type: 'with-pr',
pr: {
Expand Down
2 changes: 1 addition & 1 deletion lib/workers/repository/update/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ export async function ensurePr(
}`;
}

if (config.fetchReleaseNotes) {
if (config.fetchReleaseNotes === 'pr') {
// fetch changelogs when not already done;
await embedChangelogs(upgrades);
}
Expand Down
37 changes: 2 additions & 35 deletions lib/workers/repository/updates/branchify.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RenovateConfig, mocked, mockedFunction } from '../../../../test/util';
import { RenovateConfig, mocked } from '../../../../test/util';
import { getConfig } from '../../../config/defaults';
import * as _changelog from '../changelog';
import { branchifyUpgrades } from './branchify';
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('workers/repository/updates/branchify', () => {
});

it('no fetch changelogs', async () => {
config.fetchReleaseNotes = false;
config.fetchReleaseNotes = 'off';
flattenUpdates.mockResolvedValueOnce([
{
depName: 'foo',
Expand Down Expand Up @@ -153,38 +153,5 @@ describe('workers/repository/updates/branchify', () => {
expect(embedChangelogs).not.toHaveBeenCalled();
expect(Object.keys(res.branches)).toHaveLength(2);
});

it('fetch changelogs if required', async () => {
config.fetchReleaseNotes = true;
config.repoIsOnboarded = true;
mockedFunction(_changelog.needsChangelogs).mockReturnValueOnce(true);
flattenUpdates.mockResolvedValueOnce([
{
depName: 'foo',
branchName: 'foo',
prTitle: 'some-title',
version: '1.1.0',
groupName: 'My Group',
group: { branchName: 'renovate/{{groupSlug}}' },
},
{
depName: 'foo',
branchName: 'foo',
prTitle: 'some-title',
version: '2.0.0',
},
{
depName: 'bar',
branchName: 'bar-{{version}}',
prTitle: 'some-title',
version: '1.1.0',
groupName: 'My Group',
group: { branchName: 'renovate/my-group' },
},
]);
const res = await branchifyUpgrades(config, {});
expect(embedChangelogs).toHaveBeenCalledOnce();
expect(Object.keys(res.branches)).toHaveLength(2);
});
});
});
17 changes: 0 additions & 17 deletions lib/workers/repository/updates/branchify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { Merge } from 'type-fest';
import type { RenovateConfig, ValidationMessage } from '../../../config/types';
import { addMeta, logger, removeMeta } from '../../../logger';
import type { BranchConfig, BranchUpgradeConfig } from '../../types';
import { embedChangelogs, needsChangelogs } from '../changelog';
import { flattenUpdates } from './flatten';
import { generateBranchConfig } from './generate';

Expand Down Expand Up @@ -72,22 +71,6 @@ export async function branchifyUpgrades(
})
.reverse();

if (config.fetchReleaseNotes && config.repoIsOnboarded) {
const branches = branchUpgrades[branchName].filter((upg) =>
needsChangelogs(upg)
);
if (branches.length) {
logger.warn(
{
branches: branches.map((b) => b.branchName),
docs: 'https://docs.renovatebot.com/templates/',
},
'Fetching changelogs early is deprecated. Remove `logJSON` and `releases` from config templates. They are only allowed in `commitBody` template. See template docs for allowed templates'
);
await embedChangelogs(branches);
}
}

const branch = generateBranchConfig(branchUpgrades[branchName]);
branch.branchName = branchName;
branch.packageFiles = packageFiles;
Expand Down

0 comments on commit 6515df8

Please sign in to comment.