Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Commit

Permalink
Don't create a new draft if an existing one exists.
Browse files Browse the repository at this point in the history
RELNOTES: n/a
PiperOrigin-RevId: 411834887
Change-Id: If58818f40617a3fa3525749d90bbfadcc5a39b7d
  • Loading branch information
kjin authored and copybara-github committed Nov 23, 2021
1 parent a43ba1d commit 30cd76a
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 42 deletions.
44 changes: 33 additions & 11 deletions scripts/release/create_closure_releases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
*/

import {Change, GitClient} from './git_client';
import {GitHubClient} from './github_client';
import {DraftReleaseOptions, GitHubClient} from './github_client';

/**
* Used for testing only, so that a client class constructor can be spied on
Expand Down Expand Up @@ -120,7 +120,7 @@ function createReleaseNotes(changes: Change[]) {
// rolled back by this one via commit hash.
const rolledbackHash = rollback[1];
const matchingChange = releaseNotes.find(
change => change.hashes.some(hash => hash === rolledbackHash));
(change) => change.hashes.some((hash) => hash === rolledbackHash));
if (matchingChange) {
// We found the change that got rolled back. Changes with the same
// release notes are stored together under the same `ReleaseNotesEntry`
Expand All @@ -131,7 +131,7 @@ function createReleaseNotes(changes: Change[]) {
// notes will be dropped entirely.
// See [*], where the dropping occurs.
matchingChange.hashes =
matchingChange.hashes.filter(hash => hash !== rolledbackHash);
matchingChange.hashes.filter((hash) => hash !== rolledbackHash);
} else {
// It's possible that this change rolls back one that was skipped in the
// release notes. If that's the case, do nothing.
Expand All @@ -150,7 +150,7 @@ function createReleaseNotes(changes: Change[]) {
releaseNotes.push({
changeType: 'NONE',
noteText: `__TODO(user):__ Rollback of ${rolledbackHash}`,
hashes: [hash]
hashes: [hash],
});
}
}
Expand All @@ -161,7 +161,7 @@ function createReleaseNotes(changes: Change[]) {
const noteText = escapeGitHubMarkdown(matchedRelnotes[2].trim());
if (noteText) {
const matchingChange = releaseNotes.find(
change => change.noteText === noteText &&
(change) => change.noteText === noteText &&
change.changeType === changeType);
if (matchingChange) {
// Merge entries with the same release notes, for the sake of
Expand All @@ -184,7 +184,7 @@ function createReleaseNotes(changes: Change[]) {
for (const {changeType, heading} of RELEASE_HEADINGS) {
const formattedChangesForHeading =
releaseNotes
.filter(changeNote => changeNote.changeType === changeType)
.filter((changeNote) => changeNote.changeType === changeType)
// [*] It's possible to have an empty hashes list if a change was
// rolled back.
.filter(({hashes}) => hashes.length)
Expand Down Expand Up @@ -247,15 +247,15 @@ export async function createClosureReleases(gitHubApiToken: string) {
const commits = await git.listCommits({from, to: 'HEAD'});

// Identify the commits in which the package.json major version changed.
const pJsonVersions: Array<{version: string, changes: Change[]}> = [];
const pJsonVersions: Array<{version: string; changes: Change[]}> = [];
// We need to push a placeholder object for the last release. This helps us
// figure out whether the immediate next commit has a version bump or not.
pJsonVersions.push({version: versionAtLastRelease, changes: []});
let seenCommits: Change[] = [];
for (const commit of commits) {
const version = await getMajorVersionAtCommit(git, commit.hash);
seenCommits.push(commit);
if (!pJsonVersions.some(entry => entry.version === version)) {
if (!pJsonVersions.some((entry) => entry.version === version)) {
pJsonVersions.push({
version,
changes: seenCommits,
Expand All @@ -266,14 +266,36 @@ export async function createClosureReleases(gitHubApiToken: string) {
// Remove the placeholder object mentioned above.
pJsonVersions.shift();

// Get a list of recent release drafts. Doing so allows us to
// reuse an existing draft rather than creating a duplicate of it.
const existingDrafts = await github.getRecentDrafts();

// Draft a new GitHub release for each package.json version change seen.
for (const {version, changes} of pJsonVersions) {
const name = `Closure Library ${version}`;
const tagName = version;
const commit = changes[changes.length - 1].hash;
const body = createReleaseNotes(changes);
// Create the release
const url = await github.draftRelease({tagName, commit, name, body});

const draftReleaseOptions: DraftReleaseOptions = {
tagName,
commit,
name,
body,
};

// Find an existing draft with this name. If it exists, update it
// instead of creating a new one.
// Even though existingDrafts is not an exhaustive list of drafts,
// it's highly unlikely that a matching draft is not sufficiently
// recent to be in existingDrafts.
const existingDraftForTag =
existingDrafts.find((draft) => draft.tagName === tagName);
if (existingDraftForTag) {
draftReleaseOptions.id = existingDraftForTag.id;
}
// Create the release and print its URL.
const url = await github.draftRelease(draftReleaseOptions);
console.error(`Drafted release for ${version} at ${url}`);
}
}
Expand All @@ -284,7 +306,7 @@ if (require.main === module) {
console.error('Need GITHUB_TOKEN env var to create releases.');
process.exit(1);
}
createClosureReleases(process.env.GITHUB_TOKEN).catch(err => {
createClosureReleases(process.env.GITHUB_TOKEN).catch((err) => {
console.error(err);
process.exit(1);
});
Expand Down
65 changes: 56 additions & 9 deletions scripts/release/create_closure_releases_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import {clientImplementationsForTesting, createClosureReleases} from './create_c
import {Change, GitClient} from './git_client';
import {GitHubClient} from './github_client';

/** The fake GitHub API token to use in tests. */
/** A fake GitHub API token to use in tests. */
const FAKE_TOKEN = 'my-github-token';
/** The fake GitHub Release URL to use in tests. */
/** A fake GitHub Release URL to use in tests. */
const FAKE_RELEASE_URL = 'http://my-github-release';
/** A fake rollback hash used in several tests. */
const FAKE_ROLLBACK_HASH = '0123456701234567012345670123456701234567';
Expand Down Expand Up @@ -59,7 +59,7 @@ function stripIndentForReleaseBody(
output += strings[i] + `${subs[i]}`;
}
output += strings[strings.length - 1];
return output.trimStart().split('\n').map(str => str.trim()).join('\n');
return output.trimStart().split('\n').map((str) => str.trim()).join('\n');
}

/**
Expand All @@ -69,23 +69,26 @@ function stripIndentForReleaseBody(
* GitHubClient#getLatestRelease is called.
* @param fakeGitCommits An array of objects that describe commit data returned
* by GitClient instance methods.
* @param fakeRecentDrafts An array of recent drafts to return when
* GitHubClient#getRecentDrafts is called.
*/
function spyOnClients(
fakeLatestReleaseHash: string, fakeGitCommits: FakeGitData[]) {
fakeLatestReleaseHash: string, fakeGitCommits: FakeGitData[],
fakeRecentDrafts: Array<{tagName: string; id: number}> = []) {
// Validate that no hashes are the same in `fakeGitCommits`.
assert.strictEqual(
new Set([...fakeGitCommits.map(x => x.hash)]).size,
new Set([...fakeGitCommits.map((x) => x.hash)]).size,
fakeGitCommits.length);

// Add a `body` to objects that are missing it.
const gitCommits: Change[] =
fakeGitCommits.map(commit => Object.assign({body: ''}, commit));
fakeGitCommits.map((commit) => Object.assign({body: ''}, commit));
// Make GitClient#listCommits list commits in `fakeGitCommits`.
spyOn(GitClient.prototype, 'listCommits').and.callFake(async ({from, to}) => {
assert.strictEqual(to, 'HEAD');
// + 1 because listCommits excludes `from`.
return gitCommits.slice(
gitCommits.findIndex(commit => commit.hash === from) + 1);
gitCommits.findIndex((commit) => commit.hash === from) + 1);
});

// Make GitClient#getFile return a minimal package.json with data from
Expand All @@ -94,7 +97,7 @@ function spyOnClients(
.and.callFake(async (commitish, file) => {
assert.strictEqual(file, 'package.json');
const {pJsonVersion} =
fakeGitCommits.find(commit => commit.hash === commitish);
fakeGitCommits.find((commit) => commit.hash === commitish);
return JSON.stringify({version: pJsonVersion});
});

Expand All @@ -104,6 +107,9 @@ function spyOnClients(
// Make GitHubClient#getLatestRelease return `FAKE_RELEASE_URL`.
spyOn(GitHubClient.prototype, 'draftRelease')
.and.returnValue(Promise.resolve(FAKE_RELEASE_URL));
// Make GitHubClient#getLatestRelease return `fakeRecentDrafts`.
spyOn(GitHubClient.prototype, 'getRecentDrafts')
.and.returnValue(Promise.resolve(fakeRecentDrafts));
}

describe('createClosureReleases', () => {
Expand Down Expand Up @@ -534,7 +540,7 @@ describe('createClosureReleases', () => {
hash: '01',
pJsonVersion: '20201009.0.0',
message: 'commit 1',
body: 'RELNOTES: Stuff happened'
body: 'RELNOTES: Stuff happened',
},
{hash: '02', pJsonVersion: '20201010.0.0', message: ''},
]);
Expand All @@ -548,4 +554,45 @@ describe('createClosureReleases', () => {
`,
});
});

it('updates a draft rather than create a new one with the same tag',
async () => {
spyOnClients(
'00',
[
{hash: '00', pJsonVersion: '20201009.0.0', message: ''},
{hash: '01', pJsonVersion: '20201010.0.0', message: ''},
],
[{tagName: 'v20201010', id: 314159}]);
spyOn(clientImplementationsForTesting, 'GitHubClient').and.callThrough();
await createClosureReleases(FAKE_TOKEN);
expect(GitHubClient.prototype.getRecentDrafts).toHaveBeenCalled();
expect(GitHubClient.prototype.draftRelease).toHaveBeenCalledOnceWith({
name: 'Closure Library v20201010',
tagName: 'v20201010',
commit: '01',
body: 'No release notes.',
id: 314159,
});
});

it('creates a new draft rather than update one with a different tag',
async () => {
spyOnClients(
'00',
[
{hash: '00', pJsonVersion: '20201009.0.0', message: ''},
{hash: '01', pJsonVersion: '20201011.0.0', message: ''},
],
[{tagName: 'v20201010', id: 314159}]);
spyOn(clientImplementationsForTesting, 'GitHubClient').and.callThrough();
await createClosureReleases(FAKE_TOKEN);
expect(GitHubClient.prototype.getRecentDrafts).toHaveBeenCalled();
expect(GitHubClient.prototype.draftRelease).toHaveBeenCalledOnceWith({
name: 'Closure Library v20201011',
tagName: 'v20201011',
commit: '01',
body: 'No release notes.',
});
});
});
69 changes: 47 additions & 22 deletions scripts/release/github_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ export interface DraftReleaseOptions {
name: string;
/** The body text for the release. */
body: string;
/**
* The ID of a draft to update.
* If omitted, a new draft will be created.
*/
id?: number;
}

/**
Expand All @@ -67,12 +72,7 @@ export class GitHubClient {
* Constructs a new GitHubClient.
* @param options Options for constructing this GitHubClient.
*/
constructor({
owner,
repo,
userAgent,
token,
}: GitHubClientOptions) {
constructor({owner, repo, userAgent, token}: GitHubClientOptions) {
this.owner = owner;
this.repo = repo;
this.octokit = new Octokit({
Expand All @@ -94,25 +94,50 @@ export class GitHubClient {
}

/**
* Drafts a new GitHub Release.
* @param options Options for drafting the GitHub Release.
* @return The URL to the GitHub Web UI for managing the drafted release.
* Gets a list of drafts out of the last 30 releases.
* @return A list of recent drafts.
*/
async draftRelease({
tagName,
commit,
name,
body,
}: DraftReleaseOptions) {
const {data} = await this.octokit.repos.createRelease({
async getRecentDrafts() {
const {data} = await this.octokit.repos.listReleases({
owner: this.owner,
repo: this.repo,
tag_name: tagName,
target_commitish: commit,
name,
body,
draft: true,
});
return data.html_url;
return data.filter((release) => release.draft).map((release) => ({
tagName:
release.tag_name,
id: release.id,
}));
}

/**
* Drafts a new GitHub Release.
* @param options Options for drafting the GitHub Release.
* @return The URL to the GitHub Web UI for managing the drafted release.
*/
async draftRelease({tagName, commit, name, body, id}: DraftReleaseOptions) {
if (id) {
const {data} = await this.octokit.repos.updateRelease({
owner: this.owner,
repo: this.repo,
tag_name: tagName,
target_commitish: commit,
name,
body,
draft: true,
release_id: id,
});
return data.html_url;
} else {
const {data} = await this.octokit.repos.createRelease({
owner: this.owner,
repo: this.repo,
tag_name: tagName,
target_commitish: commit,
name,
body,
draft: true,
});
return data.html_url;
}
}
}

0 comments on commit 30cd76a

Please sign in to comment.