From 41d97e876dd3d79eb062555b94076f809bec3cb0 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Sun, 18 Jun 2023 13:58:25 +0545 Subject: [PATCH] feat(release-notes)!: support configurable fetching stage (#22781) 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 --- docs/usage/configuration-options.md | 9 ++++- .../fetch-release-notes-migration.spec.ts | 22 +++++++++++ .../custom/fetch-release-notes-migration.ts | 12 ++++++ lib/config/migrations/migrations-service.ts | 2 + lib/config/options/index.ts | 7 ++-- lib/config/types.ts | 4 +- .../repository/changelog/index.spec.ts | 21 +---------- lib/workers/repository/changelog/index.ts | 18 --------- .../repository/update/branch/index.spec.ts | 5 +-- lib/workers/repository/update/branch/index.ts | 14 +++---- .../repository/update/pr/index.spec.ts | 10 ++--- lib/workers/repository/update/pr/index.ts | 2 +- .../repository/updates/branchify.spec.ts | 37 +------------------ lib/workers/repository/updates/branchify.ts | 17 --------- 14 files changed, 66 insertions(+), 114 deletions(-) create mode 100644 lib/config/migrations/custom/fetch-release-notes-migration.spec.ts create mode 100644 lib/config/migrations/custom/fetch-release-notes-migration.ts diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 26a04b31509fe3..f48990d6c0129f 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -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: diff --git a/lib/config/migrations/custom/fetch-release-notes-migration.spec.ts b/lib/config/migrations/custom/fetch-release-notes-migration.spec.ts new file mode 100644 index 00000000000000..2e17148597eec2 --- /dev/null +++ b/lib/config/migrations/custom/fetch-release-notes-migration.spec.ts @@ -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', + } + ); + }); +}); diff --git a/lib/config/migrations/custom/fetch-release-notes-migration.ts b/lib/config/migrations/custom/fetch-release-notes-migration.ts new file mode 100644 index 00000000000000..60b7afe6eaa1a4 --- /dev/null +++ b/lib/config/migrations/custom/fetch-release-notes-migration.ts @@ -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'); + } + } +} diff --git a/lib/config/migrations/migrations-service.ts b/lib/config/migrations/migrations-service.ts index e7e3b314bb51d1..4818e0f2c2a55b 100644 --- a/lib/config/migrations/migrations-service.ts +++ b/lib/config/migrations/migrations-service.ts @@ -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'; @@ -148,6 +149,7 @@ export class MigrationsService { DatasourceMigration, RecreateClosedMigration, StabilityDaysMigration, + FetchReleaseNotesMigration, ]; static run(originalConfig: RenovateConfig): RenovateConfig { diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index f2a027fa18dcf1..ee6cf0be989c9e 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -2567,9 +2567,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, }, diff --git a/lib/config/types.ts b/lib/config/types.ts index 6136e576397228..cb7bb2be409062 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -261,7 +261,7 @@ export interface RenovateConfig vulnerabilitySeverity?: string; regexManagers?: RegExManager[]; - fetchReleaseNotes?: boolean; + fetchReleaseNotes?: FetchReleaseNotesOptions; secrets?: Record; constraints?: Record; @@ -302,6 +302,8 @@ export type UpdateType = | 'bump' | 'replacement'; +export type FetchReleaseNotesOptions = 'off' | 'branch' | 'pr'; + export type MatchStringsStrategy = 'any' | 'recursive' | 'combination'; export type MergeStrategy = diff --git a/lib/workers/repository/changelog/index.spec.ts b/lib/workers/repository/changelog/index.spec.ts index 5ad0dd54bf60c4..709103b105b098 100644 --- a/lib/workers/repository/changelog/index.spec.ts +++ b/lib/workers/repository/changelog/index.spec.ts @@ -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'); @@ -27,23 +27,4 @@ describe('workers/repository/changelog/index', () => { { logJSON: null }, ]); }); - - it('needsChangelogs', () => { - expect(needsChangelogs(partial())).toBeFalse(); - expect( - needsChangelogs( - partial({ - commitBody: '{{#if logJSON.hasReleaseNotes}}has changelog{{/if}}', - }) - ) - ).toBeFalse(); - expect( - needsChangelogs( - partial({ - commitBody: '{{#if logJSON.hasReleaseNotes}}has changelog{{/if}}', - }), - ['commitBody'] - ) - ).toBeTrue(); - }); }); diff --git a/lib/workers/repository/changelog/index.ts b/lib/workers/repository/changelog/index.ts index 8bc6c455590223..f6c6c6ca4f6881 100644 --- a/lib/workers/repository/changelog/index.ts +++ b/lib/workers/repository/changelog/index.ts @@ -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'; @@ -21,17 +17,3 @@ export async function embedChangelogs( ): Promise { 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; -} diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index fb98eda0326aa6..ebb9d62ba954cf 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -2,7 +2,6 @@ import { fs, git, mocked, - mockedFunction, partial, platform, scm, @@ -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'; @@ -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, diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index a7dafcec36b930..6654edb2bbed68 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -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'; @@ -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 ); @@ -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, diff --git a/lib/workers/repository/update/pr/index.spec.ts b/lib/workers/repository/update/pr/index.spec.ts index 06caf733c26267..7316ea6f0e5e09 100644 --- a/lib/workers/repository/update/pr/index.spec.ts +++ b/lib/workers/repository/update/pr/index.spec.ts @@ -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); @@ -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, @@ -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: { diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index ff626d34a01711..ba137dc1328a65 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -233,7 +233,7 @@ export async function ensurePr( }`; } - if (config.fetchReleaseNotes) { + if (config.fetchReleaseNotes === 'pr') { // fetch changelogs when not already done; await embedChangelogs(upgrades); } diff --git a/lib/workers/repository/updates/branchify.spec.ts b/lib/workers/repository/updates/branchify.spec.ts index 37c98be11dba29..99c0d262bb4990 100644 --- a/lib/workers/repository/updates/branchify.spec.ts +++ b/lib/workers/repository/updates/branchify.spec.ts @@ -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'; @@ -124,7 +124,7 @@ describe('workers/repository/updates/branchify', () => { }); it('no fetch changelogs', async () => { - config.fetchReleaseNotes = false; + config.fetchReleaseNotes = 'off'; flattenUpdates.mockResolvedValueOnce([ { depName: 'foo', @@ -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); - }); }); }); diff --git a/lib/workers/repository/updates/branchify.ts b/lib/workers/repository/updates/branchify.ts index 4401fb4341640d..d6d0ccf12e53aa 100644 --- a/lib/workers/repository/updates/branchify.ts +++ b/lib/workers/repository/updates/branchify.ts @@ -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'; @@ -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;