Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/changelog.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ describe("Changelog", () => {
name: "User Bot",
},
},
"https://api.github.com/apps/copilot-swe-agent": {
body: {
name: "Copilot SWE Agent",
slug: "copilot-swe-agent",
html_url: "https://github.com/apps/copilot-swe-agent",
},
},
};
fetch.__setMockResponses(usersCache);
});
Expand All @@ -222,6 +229,10 @@ describe("Changelog", () => {
});

const testCommits = [
{
commitSHA: "a0000005",
githubIssue: { user: { login: "copilot", html_url: "https://github.com/apps/copilot-swe-agent" } },
},
{
commitSHA: "a0000004",
githubIssue: { user: { login: "test-user-1" } },
Expand All @@ -236,6 +247,11 @@ describe("Changelog", () => {
const committers = await changelog.getCommitters(testCommits);

expect(committers).toEqual([
{
name: "Copilot",
slug: "copilot-swe-agent",
html_url: "https://github.com/apps/copilot-swe-agent",
},
{
login: "test-user-1",
html_url: "https://github.com/test-user-1",
Expand Down
21 changes: 16 additions & 5 deletions src/changelog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import progressBar from "./progress-bar";
import { Configuration } from "./configuration";
import findPullRequestId from "./find-pull-request-id";
import * as Git from "./git";
import GithubAPI, { GitHubUserResponse } from "./github-api";
import GithubAPI, { GitHubContributor } from "./github-api";
import { CommitInfo, Release } from "./interfaces";
import MarkdownRenderer from "./markdown-renderer";

Expand Down Expand Up @@ -124,17 +124,19 @@ export default class Changelog {
return Git.listCommits(from, to);
}

private async getCommitters(commits: CommitInfo[]): Promise<GitHubUserResponse[]> {
const committers: { [id: string]: GitHubUserResponse } = {};
private async getCommitters(commits: CommitInfo[]): Promise<GitHubContributor[]> {
const committers: { [id: string]: GitHubContributor } = {};

for (const commit of commits) {
const issue = commit.githubIssue;
const login = issue && issue.user && issue.user.login;
const user = issue && issue.user;
const login = user && user.login;
// If a list of `ignoreCommitters` is provided in the lerna.json config
// check if the current committer should be kept or not.
const shouldKeepCommiter = login && !this.ignoreCommitter(login);

if (login && shouldKeepCommiter && !committers[login]) {
committers[login] = await this.github.getUserData(login);
committers[login] = this.sanitizeCommitter(await this.github.getUserData(user));
}
}

Expand All @@ -145,6 +147,15 @@ export default class Changelog {
return this.config.ignoreCommitters.some((c: string) => c === login || login.indexOf(c) > -1);
}

private sanitizeCommitter(contributor: GitHubContributor) {
// Response for Copilot is "Copilot SWE Agent" - but we prefer "Copilot"
if (contributor.name === "Copilot SWE Agent") {
contributor.name = "Copilot";
}

return contributor;
}

private toCommitInfos(commits: Git.CommitListItem[]): CommitInfo[] {
return commits.map(commit => {
const { sha, refName, summary: message, date } = commit;
Expand Down
4 changes: 2 additions & 2 deletions src/functional/__snapshots__/markdown-full.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ exports[`createMarkdown > single project > outputs correct changelog 1`] = `
* This is the commit title for the issue (#8) ([@bot-user](https://github.com/bot-user))

#### Committers: 2
- Bot User ([@bot-user](https://github.com/bot-user))
- Bot User [Bot] ([@bot-user](https://github.com/bot-user))
- Han Solo ([@han-solo](https://github.com/han-solo))


Expand Down Expand Up @@ -175,7 +175,7 @@ exports[`createMarkdown > single tags > outputs correct changelog 1`] = `
* This is the commit title for the issue (#8) ([@bot-user](https://github.com/bot-user))

#### Committers: 2
- Bot User ([@bot-user](https://github.com/bot-user))
- Bot User [Bot] ([@bot-user](https://github.com/bot-user))
- Han Solo ([@han-solo](https://github.com/han-solo))


Expand Down
18 changes: 13 additions & 5 deletions src/functional/markdown-full.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,55 +163,63 @@ const usersCache = {
"https://api.github.com/users/luke": {
body: {
login: "luke",
type: "User",
html_url: "https://github.com/luke",
name: "Luke Skywalker",
},
},
"https://api.github.com/users/princess-leia": {
body: {
login: "princess-leia",
type: "User",
html_url: "https://github.com/princess-leia",
name: "Princess Leia Organa",
},
},
"https://api.github.com/users/vader": {
body: {
login: "vader",
type: "User",
html_url: "https://github.com/vader",
name: "Darth Vader",
},
},
"https://api.github.com/users/gtarkin": {
body: {
login: "gtarkin",
type: "User",
html_url: "https://github.com/gtarkin",
name: "Governor Tarkin",
},
},
"https://api.github.com/users/han-solo": {
body: {
login: "han-solo",
type: "User",
html_url: "https://github.com/han-solo",
name: "Han Solo",
},
},
"https://api.github.com/users/chewbacca": {
body: {
login: "chewbacca",
type: "User",
html_url: "https://github.com/chewbacca",
name: "Chwebacca",
},
},
"https://api.github.com/users/rd-d2": {
body: {
login: "rd-d2",
type: "User",
html_url: "https://github.com/rd-d2",
name: "R2-D2",
},
},
"https://api.github.com/users/c-3po": {
body: {
login: "c-3po",
type: "User",
html_url: "https://github.com/c-3po",
name: "C-3PO",
},
Expand Down Expand Up @@ -327,7 +335,7 @@ describe("createMarkdown", () => {

describe("ignore config", () => {
it("ignores PRs from bot users even if they were not the (merge) committer", async () => {
git.changedPaths.mockImplementation((sha) => {
git.changedPaths.mockImplementation(sha => {
return listOfPackagesForEachCommit[sha];
});
git.lastTag.mockImplementation(() => "v8.0.0");
Expand All @@ -351,7 +359,7 @@ describe("createMarkdown", () => {

describe("single tags", () => {
it("outputs correct changelog", async () => {
git.changedPaths.mockImplementation((sha) => listOfPackagesForEachCommit[sha]);
git.changedPaths.mockImplementation(sha => listOfPackagesForEachCommit[sha]);
git.lastTag.mockImplementation(() => "v8.0.0");
git.listCommits.mockImplementation(() => listOfCommits);
git.listTagNames.mockImplementation(() => listOfTags);
Expand All @@ -371,7 +379,7 @@ describe("createMarkdown", () => {

describe("multiple tags", () => {
it("outputs correct changelog", async () => {
git.changedPaths.mockImplementation((sha) => listOfPackagesForEachCommit[sha]);
git.changedPaths.mockImplementation(sha => listOfPackagesForEachCommit[sha]);
git.lastTag.mockImplementation(() => "v8.0.0");
git.listCommits.mockImplementation(() => [
{
Expand Down Expand Up @@ -424,7 +432,7 @@ describe("createMarkdown", () => {

describe("single project", () => {
it("outputs correct changelog", async () => {
git.changedPaths.mockImplementation((sha) => listOfFileForEachCommit[sha]);
git.changedPaths.mockImplementation(sha => listOfFileForEachCommit[sha]);
git.lastTag.mockImplementation(() => "v8.0.0");
git.listCommits.mockImplementation(() => listOfCommits);
git.listTagNames.mockImplementation(() => listOfTags);
Expand All @@ -449,7 +457,7 @@ describe("createMarkdown", () => {
documentation_url: "https://developer.github.com/v3",
};
beforeEach(async () => {
git.changedPaths.mockImplementation((sha) => listOfFileForEachCommit[sha]);
git.changedPaths.mockImplementation(sha => listOfFileForEachCommit[sha]);
git.lastTag.mockImplementation(() => "v8.0.0");
git.listCommits.mockImplementation(() => listOfCommits);
git.listTagNames.mockImplementation(() => listOfTags);
Expand Down
7 changes: 5 additions & 2 deletions src/github-api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ describe("github api", function () {
};
expect(github.getBaseIssueUrl("foo")).toEqual(`https://github.host.com/foo/issues/`);

github.getUserData("foo");
github.getUserData({ login: "foo", html_url: "" });
expect(fetchedUrl).toEqual(`https://api.github.host.com/users/foo`);

github.getIssueData("foo", "2");
expect(fetchedUrl).toEqual(`https://api.github.host.com/repos/foo/issues/2`);

github.getUserData({ login: "Copilot", html_url: "https://github.com/apps/copilot-swe-agent" });
expect(fetchedUrl).toEqual(`https://api.github.host.com/apps/copilot-swe-agent`);

delete process.env.GITHUB_DOMAIN;

process.env.GITHUB_API_URL = "https://api.github.host2.com";
Expand All @@ -36,7 +39,7 @@ describe("github api", function () {
};
expect(github.getBaseIssueUrl("foo")).toEqual(`https://github.com/foo/issues/`);

github.getUserData("foo");
github.getUserData({ login: "foo", html_url: "" });
expect(fetchedUrl).toEqual(`https://api.github.host2.com/users/foo`);

github.getIssueData("foo", "2");
Expand Down
32 changes: 24 additions & 8 deletions src/github-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,22 @@ const path = require("path");
import ConfigurationError from "./configuration-error";
import fetch from "./fetch";

export interface GitHubUserResponse {
login: string;
interface GitHubContributorBase {
name: string;
html_url: string;
}

export interface GithubUserInfo extends GitHubContributorBase {
login: string;
type: string; // "Bot" | "User"
}

export interface GithubAppInfo extends GitHubContributorBase {
slug: string;
}

export type GitHubContributor = GithubUserInfo | GithubAppInfo;

export interface GitHubIssueResponse {
number: number;
title: string;
Expand All @@ -18,10 +28,7 @@ export interface GitHubIssueResponse {
labels: Array<{
name: string;
}>;
user: {
login: string;
html_url: string;
};
user: GithubUserInfo;
}

export interface Options {
Expand Down Expand Up @@ -54,9 +61,18 @@ export default class GithubAPI {
return this._fetch(`${prefix}/repos/${repo}/issues/${issue}`);
}

public async getUserData(login: string): Promise<GitHubUserResponse> {
public async getUserData(userInfo: Pick<GithubUserInfo, "login" | "html_url">): Promise<GitHubContributor> {
let login = userInfo.login;
let path = "users";

// github API itself does not tell if contributor is an app. Best guess was to
// check the `html_url` for the `/apps/` segment
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not use type which is set to Bot for Copilot?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, apps are a sub-type of Bot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not really sure that Github knows what it is doing. That seems very much like an impl without spec: "we need it, make it happen, doesn't matter how".

if (userInfo.html_url && userInfo.html_url.includes("/apps/")) {
path = "apps";
login = userInfo.html_url.split("/").pop() as string;
}
const prefix = process.env.GITHUB_API_URL || `https://api.${this.github}`;
return this._fetch(`${prefix}/users/${login}`);
return await this._fetch(`${prefix}/${path}/${login}`);
}

private async _fetch(url: string): Promise<any> {
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { GitHubIssueResponse, GitHubUserResponse } from "./github-api";
import { GitHubIssueResponse, GitHubContributor } from "./github-api";

export interface CommitInfo {
commitSHA: string;
Expand All @@ -15,5 +15,5 @@ export interface Release {
name: string;
date: string;
commits: CommitInfo[];
contributors?: GitHubUserResponse[];
contributors?: GitHubContributor[];
}
18 changes: 17 additions & 1 deletion src/markdown-renderer.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { GithubAppInfo, GithubUserInfo } from "./github-api";
import { CommitInfo, Release } from "./interfaces";
import MarkdownRenderer from "./markdown-renderer";

Expand Down Expand Up @@ -120,12 +121,14 @@ describe("MarkdownRenderer", () => {
const user1 = {
login: "hzoo",
name: "",
type: "User",
html_url: "https://github.com/hzoo",
};

const user2 = {
login: "Turbo87",
name: "Tobias Bieniek",
type: "User",
html_url: "https://github.com/Turbo87",
};

Expand All @@ -140,6 +143,7 @@ describe("MarkdownRenderer", () => {
const result = renderer().renderContributor({
login: "foo",
name: "",
type: "User",
html_url: "http://github.com/foo",
});

Expand All @@ -150,11 +154,23 @@ describe("MarkdownRenderer", () => {
const result = renderer().renderContributor({
login: "foo",
name: "Foo Bar",
type: "User",
html_url: "http://github.com/foo",
});

expect(result).toEqual("Foo Bar ([@foo](http://github.com/foo))");
});

it(`renders Copilot`, () => {
const result = renderer().renderContributor({
login: "Copilot",
name: "Copilot",
slug: "copilot-swe-agent",
html_url: "https://github.com/apps/copilot-swe-agent",
} as GithubAppInfo);

expect(result).toEqual("Copilot [Bot] ([@copilot-swe-agent](https://github.com/apps/copilot-swe-agent))");
});
});

describe("groupByCategory", () => {
Expand Down Expand Up @@ -230,7 +246,7 @@ describe("MarkdownRenderer", () => {
categories: [":rocket: New Feature"],
},
],
contributors: [{ name: "Henry", login: "hzoo", html_url: "http://hzoo.com" }],
contributors: [{ name: "Henry", login: "hzoo", type: "User", html_url: "http://hzoo.com" }],
};
const options = {
categories: [":rocket: New Feature"],
Expand Down
Loading