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: defer commitBody compile #17108

Merged
merged 8 commits into from Aug 11, 2022
Merged
2 changes: 2 additions & 0 deletions docs/usage/templates.md
Expand Up @@ -13,6 +13,8 @@ You can recognize templates when you see strings like `{{depName}}` in configura
Below you can find lists of fields/values that you can use in templates.
Some are configuration options passed through, while others are generated as part of Renovate's run.

`logJSON` and `releases` are only allowed in `commitBody` template.

## Exposed config options

<!-- Autogenerate in https://github.com/renovatebot/renovate -->
Expand Down
8 changes: 8 additions & 0 deletions lib/workers/repository/changelog/index.spec.ts
Expand Up @@ -36,6 +36,14 @@ describe('workers/repository/changelog/index', () => {
commitBody: '{{#if logJSON.hasReleaseNotes}}has changelog{{/if}}',
})
)
).toBeFalse();
expect(
needsChangelogs(
partial<BranchUpgradeConfig>({
commitBody: '{{#if logJSON.hasReleaseNotes}}has changelog{{/if}}',
}),
['commitBody']
)
).toBeTrue();
});
});
12 changes: 9 additions & 3 deletions lib/workers/repository/changelog/index.ts
Expand Up @@ -6,7 +6,9 @@ import {
import type { BranchUpgradeConfig } from '../../types';
import { getChangeLogJSON } from '../update/pr/changelog';

async function embedChangelog(upgrade: BranchUpgradeConfig): Promise<void> {
export async function embedChangelog(
upgrade: BranchUpgradeConfig
): Promise<void> {
// getChangeLogJSON returns null on error, so don't try again
if (upgrade.logJSON !== undefined) {
return;
Expand All @@ -20,8 +22,12 @@ export async function embedChangelogs(
await pMap(branches, embedChangelog, { concurrency: 10 });
}

export function needsChangelogs(upgrade: BranchUpgradeConfig): boolean {
for (const field of exposedConfigOptions) {
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;
viceice marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
4 changes: 3 additions & 1 deletion lib/workers/repository/index.ts
Expand Up @@ -51,7 +51,9 @@ export async function renovateRepository(
addSplit('onboarding');
if (config.fetchReleaseNotes && config.repoIsOnboarded) {
logger.info('Fetching changelogs');
await embedChangelogs(branches);
for (const branch of branches) {
await embedChangelogs(branch.upgrades);
}
}
addSplit('changelogs');
const res = await updateRepo(config, branches);
Expand Down
3 changes: 2 additions & 1 deletion lib/workers/repository/update/branch/index.spec.ts
Expand Up @@ -46,6 +46,7 @@ jest.mock('./automerge');
jest.mock('./commit');
jest.mock('../pr');
jest.mock('../pr/automerge');
jest.mock('../../changelog');
jest.mock('../../../../util/exec');
jest.mock('../../../../util/merge-confidence');
jest.mock('../../../../util/sanitize');
Expand Down Expand Up @@ -656,13 +657,13 @@ describe('workers/repository/update/branch/index', () => {
...config,
ignoreTests: true,
prCreation: 'not-pending',
commitBody: '[skip-ci]',
})
).toEqual({
branchExists: true,
prNo: undefined,
result: 'pending',
});

expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0);
viceice marked this conversation as resolved.
Show resolved Hide resolved
expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
});
Expand Down
16 changes: 16 additions & 0 deletions lib/workers/repository/update/branch/index.ts
Expand Up @@ -40,8 +40,10 @@ import {
isActiveConfidenceLevel,
satisfiesConfidenceLevel,
} from '../../../../util/merge-confidence';
import * as template from '../../../../util/template';
import { Limit, isLimitReached } from '../../../global/limits';
import { BranchConfig, BranchResult, PrBlockedBy } from '../../../types';
// import { embedChangelog, needsChangelogs } from '../../changelog';
import { ensurePr, getPlatformPrOptions, updatePrDebugData } from '../pr';
import { checkAutoMerge } from '../pr/automerge';
import { getPrBody } from '../pr/body';
Expand Down Expand Up @@ -477,6 +479,20 @@ export async function processBranch(
}
}

// compile commit message with body, which maybe needs changelogs
if (config.commitBody) {
// TODO: defer fetching changelogs (#17020)
// if (config.fetchReleaseNotes && needsChangelogs(config, ['commitBody'])) {
// await embedChangelog(config);
// }
config.commitMessage = `${config.commitMessage!}\n\n${template.compile(
config.commitBody,
config
)}`;

logger.trace(`commitMessage: ` + JSON.stringify(config.commitMessage));
}

const commitSha = await commitFilesToBranch(config);
// istanbul ignore if
if (branchPr && platform.refreshPr) {
Expand Down
1 change: 1 addition & 0 deletions lib/workers/repository/update/pr/index.spec.ts
Expand Up @@ -24,6 +24,7 @@ import * as _participants from './participants';
import { ensurePr } from '.';

jest.mock('../../../../util/git');
jest.mock('../../changelog');

jest.mock('../../../global/limits');
const limits = mocked(_limits);
Expand Down
7 changes: 7 additions & 0 deletions lib/workers/repository/update/pr/index.ts
Expand Up @@ -27,6 +27,7 @@ import type {
BranchUpgradeConfig,
PrBlockedBy,
} from '../../../types';
// import { embedChangelogs } from '../../changelog';
import { resolveBranchStatus } from '../branch/status-checks';
import { getPrBody } from './body';
import { ChangeLogError } from './changelog/types';
Expand Down Expand Up @@ -194,6 +195,12 @@ export async function ensurePr(
}`;
}

// TODO: defer fetching changelogs (#17020)
// if (config.fetchReleaseNotes) {
// // fetch changelogs when not already done;
// await embedChangelogs(upgrades);
// }

// Get changelog and then generate template strings
for (const upgrade of upgrades) {
// TODO: types (#7154)
Expand Down
@@ -1,7 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`workers/repository/updates/generate generateBranchConfig() adds commit message body 1`] = `"Update dependency some-dep to v1.2.0\\n\\n[skip-ci]"`;

exports[`workers/repository/updates/generate generateBranchConfig() handles @types specially (reversed) 1`] = `
Object {
"addLabels": Array [],
Expand Down
13 changes: 9 additions & 4 deletions lib/workers/repository/updates/branchify.ts
Expand Up @@ -73,11 +73,16 @@ export async function branchifyUpgrades(
.reverse();

if (config.fetchReleaseNotes && config.repoIsOnboarded) {
const branches = branchUpgrades[branchName].filter(needsChangelogs);
const branches = branchUpgrades[branchName].filter((upg) =>
needsChangelogs(upg)
);
if (branches.length) {
logger.info(
{ branches: branches.map((b) => b.branchName) },
'Fetching changelogs early'
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);
}
Expand Down
3 changes: 1 addition & 2 deletions lib/workers/repository/updates/generate.spec.ts
Expand Up @@ -679,8 +679,7 @@ describe('workers/repository/updates/generate', () => {
} as BranchUpgradeConfig,
];
const res = generateBranchConfig(branch);
expect(res.commitMessage).toMatchSnapshot();
expect(res.commitMessage).toContain('\n');
expect(res.commitMessage).toBe('Update dependency some-dep to v1.2.0');
});

it('supports manual prTitle', () => {
Expand Down
7 changes: 1 addition & 6 deletions lib/workers/repository/updates/generate.ts
Expand Up @@ -223,12 +223,7 @@ export function generateBranchConfig(
splitMessage[0] = splitMessage[0].toLowerCase();
upgrade.commitMessage = splitMessage.join('\n');
}
if (upgrade.commitBody) {
upgrade.commitMessage = `${upgrade.commitMessage}\n\n${template.compile(
upgrade.commitBody,
upgrade
)}`;
}

logger.trace(`commitMessage: ` + JSON.stringify(upgrade.commitMessage));
if (upgrade.prTitle) {
upgrade.prTitle = template.compile(upgrade.prTitle, upgrade);
Expand Down