diff --git a/src/bin/code-suggester.ts b/src/bin/code-suggester.ts index c8b5d11d..510349cb 100644 --- a/src/bin/code-suggester.ts +++ b/src/bin/code-suggester.ts @@ -97,6 +97,11 @@ yargs default: [], type: 'array', }, + 'files-per-commit': { + describe: 'Number of files per commit. Defaults to 100', + default: 100, + type: 'number', + }, }) .command(REVIEW_PR_COMMAND, 'Review an open pull request', { 'upstream-repo': { diff --git a/src/bin/workflow.ts b/src/bin/workflow.ts index bc0e0be4..97b7064e 100644 --- a/src/bin/workflow.ts +++ b/src/bin/workflow.ts @@ -42,6 +42,7 @@ export function coerceUserCreatePullRequestOptions(): CreatePullRequestUserOptio fork: yargs.argv.fork as boolean, labels: yargs.argv.labels as string[], logger, + filesPerCommit: yargs.argv.filesPerCommit as number, }; } diff --git a/src/default-options-handler.ts b/src/default-options-handler.ts index 090cb578..db66d569 100644 --- a/src/default-options-handler.ts +++ b/src/default-options-handler.ts @@ -48,6 +48,7 @@ export function addPullRequestDefaults( ? options.primary : DEFAULT_PRIMARY_BRANCH, maintainersCanModify: options.maintainersCanModify === false ? false : true, + filesPerCommit: options.filesPerCommit, }; return pullRequestSettings; } diff --git a/src/github/commit-and-push.ts b/src/github/commit-and-push.ts index 48b81c2c..f287a0a6 100644 --- a/src/github/commit-and-push.ts +++ b/src/github/commit-and-push.ts @@ -21,6 +21,9 @@ import { } from '../types'; import {Octokit} from '@octokit/rest'; import {logger} from '../logger'; +import {createCommit} from './create-commit'; + +const DEFAULT_FILES_PER_COMMIT = 100; /** * Generate and return a GitHub tree object structure @@ -53,6 +56,15 @@ export function generateTreeObjects(changes: Changes): TreeObject[] { return tree; } +function* inGroupsOf( + all: T[], + groupSize: number +): Generator { + for (let i = 0; i < all.length; i += groupSize) { + yield all.slice(i, i + groupSize); + } +} + /** * Upload and create a remote GitHub tree * and resolves with the new tree SHA. @@ -91,37 +103,6 @@ export async function createTree( return treeSha; } -/** - * Create a commit with a repo snapshot SHA on top of the reference HEAD - * and resolves with the SHA of the commit. - * Rejects if GitHub V3 API fails with the GitHub error response - * @param {Octokit} octokit The authenticated octokit instance - * @param {RepoDomain} origin the the remote repository to push changes to - * @param {string} refHead the base of the new commit(s) - * @param {string} treeSha the tree SHA that this commit will point to - * @param {string} message the message of the new commit - * @returns {Promise} the new commit SHA - */ -export async function createCommit( - octokit: Octokit, - origin: RepoDomain, - refHead: string, - treeSha: string, - message: string -): Promise { - const commitData = ( - await octokit.git.createCommit({ - owner: origin.owner, - repo: origin.repo, - message, - tree: treeSha, - parents: [refHead], - }) - ).data; - logger.info(`Successfully created commit. See commit at ${commitData.url}`); - return commitData.sha; -} - /** * Update a reference to a SHA * Rejects if GitHub V3 API fails with the GitHub error response @@ -166,19 +147,27 @@ export async function commitAndPush( changes: Changes, originBranch: BranchDomain, commitMessage: string, - force: boolean + force: boolean, + filesPerCommit: number = DEFAULT_FILES_PER_COMMIT ) { try { const tree = generateTreeObjects(changes); - const treeSha = await createTree(octokit, originBranch, refHead, tree); - const commitSha = await createCommit( - octokit, - originBranch, - refHead, - treeSha, - commitMessage - ); - await updateRef(octokit, originBranch, commitSha, force); + for (const treeGroup of inGroupsOf(tree, filesPerCommit)) { + const treeSha = await createTree( + octokit, + originBranch, + refHead, + treeGroup + ); + refHead = await createCommit( + octokit, + originBranch, + refHead, + treeSha, + commitMessage + ); + } + await updateRef(octokit, originBranch, refHead, force); } catch (err) { ( err as Error diff --git a/src/github/create-commit.ts b/src/github/create-commit.ts new file mode 100644 index 00000000..7529291a --- /dev/null +++ b/src/github/create-commit.ts @@ -0,0 +1,48 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {Octokit} from '@octokit/rest'; +import {RepoDomain} from '../types'; +import {logger} from '../logger'; + +/** + * Create a commit with a repo snapshot SHA on top of the reference HEAD + * and resolves with the SHA of the commit. + * Rejects if GitHub V3 API fails with the GitHub error response + * @param {Octokit} octokit The authenticated octokit instance + * @param {RepoDomain} origin the the remote repository to push changes to + * @param {string} refHead the base of the new commit(s) + * @param {string} treeSha the tree SHA that this commit will point to + * @param {string} message the message of the new commit + * @returns {Promise} the new commit SHA + */ +export async function createCommit( + octokit: Octokit, + origin: RepoDomain, + refHead: string, + treeSha: string, + message: string +): Promise { + const commitData = ( + await octokit.git.createCommit({ + owner: origin.owner, + repo: origin.repo, + message, + tree: treeSha, + parents: [refHead], + }) + ).data; + logger.info(`Successfully created commit. See commit at ${commitData.url}`); + return commitData.sha; +} diff --git a/src/index.ts b/src/index.ts index 6a906599..bd4ad371 100644 --- a/src/index.ts +++ b/src/index.ts @@ -189,7 +189,8 @@ async function createPullRequest( changes, originBranch, gitHubConfigs.message, - gitHubConfigs.force + gitHubConfigs.force, + gitHubConfigs.filesPerCommit ); const description: Description = { diff --git a/src/types.ts b/src/types.ts index e4572d4f..446634fc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -101,6 +101,8 @@ export interface CreatePullRequestUserOptions { draft?: boolean; // Optional logger to set logger?: Logger; + // Optional number of files per commit + filesPerCommit?: number; } /** @@ -125,6 +127,8 @@ export interface CreatePullRequest { primary: string; // Whether or not maintainers can modify the PR. maintainersCanModify: boolean; + // Optional number of files per commit + filesPerCommit?: number; } /** diff --git a/test/cli.ts b/test/cli.ts index 47053b57..8570970c 100644 --- a/test/cli.ts +++ b/test/cli.ts @@ -91,6 +91,7 @@ describe('Mapping pr yargs to create PR options', () => { fork: true, labels: ['automerge'], logger: console, + filesPerCommit: 100, }; sandbox.stub(yargs, 'argv').value({_: ['pr'], ...options}); assert.deepStrictEqual(coerceUserCreatePullRequestOptions(), options); diff --git a/test/commit-and-push.ts b/test/commit-and-push.ts index 4318a904..8afc8e86 100644 --- a/test/commit-and-push.ts +++ b/test/commit-and-push.ts @@ -20,7 +20,9 @@ import {octokit, setup} from './util'; import * as sinon from 'sinon'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; import * as handler from '../src/github/commit-and-push'; +import * as createCommitModule from '../src/github/create-commit'; import {Changes, FileData, TreeObject, RepoDomain} from '../src/types'; +import {createCommit} from '../src/github/create-commit'; type GetCommitResponse = GetResponseTypeFromEndpointMethod< typeof octokit.git.getCommit @@ -194,13 +196,7 @@ describe('Commit', () => { .stub(octokit.git, 'createCommit') .resolves(createCommitResponse); // tests - const sha = await handler.createCommit( - octokit, - origin, - head, - treeSha, - message - ); + const sha = await createCommit(octokit, origin, head, treeSha, message); assert.strictEqual(sha, createCommitResponse.data.sha); sandbox.assert.calledOnceWithExactly(stubCreateCommit, { owner: origin.owner, @@ -285,6 +281,10 @@ describe('Commit and push function', async () => { sandbox.restore(); }); it('When everything works it calls functions with correct parameter values', async () => { + changes.set('foo.txt', { + mode: '100755', + content: 'some file content', + }); // setup const stubGetCommit = sandbox .stub(octokit.git, 'getCommit') @@ -313,7 +313,14 @@ describe('Commit and push function', async () => { sandbox.assert.calledWithExactly(stubCreateTree, { owner: origin.owner, repo: origin.repo, - tree: [], + tree: [ + { + path: 'foo.txt', + mode: '100755', + type: 'blob', + content: 'some file content', + }, + ], base_tree: getCommitResponse.data.tree.sha, }); sandbox.assert.calledOnceWithExactly(stubCreateCommit, { @@ -342,6 +349,57 @@ describe('Commit and push function', async () => { sandbox.stub(octokit.git, 'getCommit').resolves(getCommitResponse); sandbox.stub(octokit.git, 'createTree').rejects(error); // tests - await assert.rejects(handler.createTree(octokit, origin, '', []), error); + await assert.rejects( + handler.createTree(octokit, origin, '', [ + {path: 'foo.txt', type: 'blob', mode: '100755'}, + ]), + error + ); + }); + it('groups files into batches', async () => { + for (let i = 0; i < 10; i++) { + changes.set(`path${i}`, { + mode: '100755', + content: 'some file content', + }); + } + sandbox.stub(octokit.git, 'getCommit').resolves(getCommitResponse); + const createCommitStub = sandbox + .stub(createCommitModule, 'createCommit') + .resolves('commitsha1') + .onSecondCall() + .resolves('commitsha2'); + const createTreeStub = sandbox.stub(octokit.git, 'createTree').resolves({ + data: { + sha: 'createdsha1', + url: 'unused', + truncated: false, + tree: [ + { + path: 'path1', + type: 'blob', + mode: '100755', + }, + ], + }, + headers: {}, + status: 201, + url: 'unused', + }); + const updateRefStub = sandbox.stub(octokit.git, 'updateRef').resolves(); + + await handler.commitAndPush( + octokit, + oldHeadSha, + changes, + {branch: branchName, ...origin}, + message, + true, + 6 + ); + + sinon.assert.calledTwice(createTreeStub); + sinon.assert.calledTwice(createCommitStub); + sinon.assert.calledOnce(updateRefStub); }); }); diff --git a/test/main-pr-option-defaults.ts b/test/main-pr-option-defaults.ts index b88aff02..bc537c4f 100644 --- a/test/main-pr-option-defaults.ts +++ b/test/main-pr-option-defaults.ts @@ -48,6 +48,7 @@ describe('addPullRequestDefaults', () => { message: 'chore: custom description', primary: 'main', maintainersCanModify: true, + filesPerCommit: undefined, }); const upstreamAndPrimary: CreatePullRequestUserOptions = { upstreamOwner: 'owner', @@ -68,6 +69,7 @@ describe('addPullRequestDefaults', () => { message: 'chore: custom description', primary: 'non-default-primary-branch', maintainersCanModify: true, + filesPerCommit: undefined, }); const upstreamAndPrDescription: CreatePullRequestUserOptions = { upstreamOwner: 'owner', @@ -87,6 +89,7 @@ describe('addPullRequestDefaults', () => { message: 'chore: custom code suggestions message', primary: 'main', maintainersCanModify: true, + filesPerCommit: undefined, }); }); it("Uses all of user's provided options", () => { @@ -100,6 +103,7 @@ describe('addPullRequestDefaults', () => { message: 'chore: code suggestions custom commit message', primary: 'non-default-primary-branch', maintainersCanModify: false, + filesPerCommit: 10, }; const gitHubPr = addPullRequestDefaults(options); assert.deepStrictEqual(gitHubPr, options);