Merge pull request #1135 from intuit/next-improvements

Next improvements
intuit · Apr 7, 2020 · 13063c1 · 13063c1
2 parents 3410f29 + 97f3fd6
commit 13063c1
Show file tree

Hide file tree

Showing 12 changed files with 194 additions and 95 deletions.
diff --git a/docs/pages/build-platforms/travis.md b/docs/pages/build-platforms/travis.md
@@ -9,11 +9,10 @@ The following config declares the `deploy` job that run on all branches. The job
 
 ```yaml
 language: node_js
-node_js: '10'
-env:
-  global:
-    - GIT_NAME="Andrew Lisowski"
-    - GIT_EMAIL="lisowski54@gmail.com"
+node_js: "10"
+
+git:
+  depth: false
 
 script:
   - yarn lint
@@ -44,7 +43,7 @@ To fix this add the following lines to your `.travis.yml`
 ```yml
 before_deploy:
   - if [ "$TRAVIS_BRANCH" == "master" ];then
-      git checkout master;
+    git checkout master;
     fi;
 ```
 

diff --git a/packages/core/package.json b/packages/core/package.json
@@ -55,7 +55,7 @@
     "fast-glob": "^3.1.1",
     "fp-ts": "^2.5.3",
     "fromentries": "^1.2.0",
-    "gitlogplus": "^3.1.2",
+    "gitlog": "^3.3.4",
     "https-proxy-agent": "^5.0.0",
     "import-cwd": "^3.0.0",
     "import-from": "^3.0.0",

diff --git a/packages/core/src/__tests__/__snapshots__/git.test.ts.snap b/packages/core/src/__tests__/__snapshots__/git.test.ts.snap
@@ -10,6 +10,34 @@ Object {
     "<PROJECT_ROOT>/packages/core/src/__tests__/auto.test.ts",
     "<PROJECT_ROOT>/packages/core/src/auto.ts",
     "<PROJECT_ROOT>/plugins/git-tag/src/index.ts",
+    "<PROJECT_ROOT>/CHANGELOG.md",
+    "<PROJECT_ROOT>/Formula/auto.rb",
+    "<PROJECT_ROOT>/lerna.json",
+    "<PROJECT_ROOT>/packages/cli/package.json",
+    "<PROJECT_ROOT>/packages/core/CHANGELOG.md",
+    "<PROJECT_ROOT>/packages/core/package.json",
+    "<PROJECT_ROOT>/packages/core/src/__tests__/release.test.ts",
+    "<PROJECT_ROOT>/packages/core/src/release.ts",
+    "<PROJECT_ROOT>/packages/core/src/utils/__tests__/load-plugin-canary.test.ts",
+    "<PROJECT_ROOT>/packages/core/src/utils/__tests__/load-plugin-windows.test.ts",
+    "<PROJECT_ROOT>/packages/core/src/utils/load-plugins.ts",
+    "<PROJECT_ROOT>/plugins/all-contributors/package.json",
+    "<PROJECT_ROOT>/plugins/chrome/package.json",
+    "<PROJECT_ROOT>/plugins/conventional-commits/package.json",
+    "<PROJECT_ROOT>/plugins/crates/package.json",
+    "<PROJECT_ROOT>/plugins/first-time-contributor/package.json",
+    "<PROJECT_ROOT>/plugins/git-tag/package.json",
+    "<PROJECT_ROOT>/plugins/gradle/package.json",
+    "<PROJECT_ROOT>/plugins/jira/package.json",
+    "<PROJECT_ROOT>/plugins/maven/package.json",
+    "<PROJECT_ROOT>/plugins/npm/package.json",
+    "<PROJECT_ROOT>/plugins/omit-commits/package.json",
+    "<PROJECT_ROOT>/plugins/omit-release-notes/package.json",
+    "<PROJECT_ROOT>/plugins/released/package.json",
+    "<PROJECT_ROOT>/plugins/s3/package.json",
+    "<PROJECT_ROOT>/plugins/slack/package.json",
+    "<PROJECT_ROOT>/plugins/twitter/package.json",
+    "<PROJECT_ROOT>/plugins/upload-assets/package.json",
   ],
   "hash": "024c66aa21cff5947957ed3bdfb4a616aa3f0046",
   "subject": "Merge pull request #1018 from intuit/exit

diff --git a/packages/core/src/__tests__/__snapshots__/release.test.ts.snap b/packages/core/src/__tests__/__snapshots__/release.test.ts.snap
@@ -289,6 +289,49 @@ Array [
 ]
 `;
 
+exports[`Release getCommits should not omit commits in next branch 1`] = `
+Array [
+  Object {
+    "authorEmail": "adam@dierkens.com",
+    "authorName": "Adam Dierkens",
+    "authors": Array [
+      Object {
+        "email": "adam@dierkens.com",
+        "name": "Adam Dierkens",
+      },
+    ],
+    "files": Array [],
+    "hash": "foo",
+    "labels": Array [],
+    "pullRequest": Object {
+      "number": 124,
+    },
+    "subject": "Feature",
+  },
+  Object {
+    "authorEmail": "adam@dierkens.com",
+    "authorName": "Adam Dierkens",
+    "authors": Array [
+      Object {
+        "email": "adam@dierkens.com",
+        "hash": "1a2b",
+        "name": "Adam Dierkens",
+      },
+    ],
+    "files": Array [],
+    "hash": "1a2b",
+    "labels": Array [
+      "skip-release",
+      "minor",
+    ],
+    "pullRequest": Object {
+      "number": 123,
+    },
+    "subject": "I wasn't released previously",
+  },
+]
+`;
+
 exports[`Release getCommits should not resolve authors with no PR commits 1`] = `
 Array [
   Object {

diff --git a/packages/core/src/__tests__/auto-make-changelog.test.ts b/packages/core/src/__tests__/auto-make-changelog.test.ts
@@ -50,16 +50,17 @@ jest.mock("@octokit/rest", () => {
 });
 
 // @ts-ignore
-jest.mock("gitlogplus", () => (a, cb) => {
-  cb(undefined, [
-    {
-      rawBody: "foo",
-    },
-    {
-      rawBody: "foo",
-    },
-  ]);
-});
+jest.mock("gitlog", () => ({
+  gitlogPromise: () =>
+    Promise.resolve([
+      {
+        rawBody: "foo",
+      },
+      {
+        rawBody: "foo",
+      },
+    ]),
+}));
 
 describe("Auto", () => {
   test("should add to changelog", async () => {

diff --git a/packages/core/src/__tests__/auto.test.ts b/packages/core/src/__tests__/auto.test.ts
@@ -56,18 +56,19 @@ jest.mock("@octokit/rest", () => {
 });
 
 // @ts-ignore
-jest.mock("gitlogplus", () => (a, cb) => {
-  cb(undefined, [
-    {
-      rawBody: "foo",
-      hash: "123",
-    },
-    {
-      rawBody: "foo",
-      hash: "456",
-    },
-  ]);
-});
+jest.mock("gitlog", () => ({
+  gitlogPromise: () =>
+    Promise.resolve([
+      {
+        rawBody: "foo",
+        hash: "123",
+      },
+      {
+        rawBody: "foo",
+        hash: "456",
+      },
+    ]),
+}));
 
 describe("Auto", () => {
   beforeEach(() => {

diff --git a/packages/core/src/__tests__/release.test.ts b/packages/core/src/__tests__/release.test.ts
@@ -394,6 +394,40 @@ describe("Release", () => {
       expect(await gh.getCommits("12345", "1234")).toMatchSnapshot();
     });
 
+    test("should not omit commits in next branch", async () => {
+      const gh = new Release(git);
+
+      jest.spyOn(console, "log").mockImplementationOnce(() => {});
+      getLatestReleaseInfo.mockReturnValueOnce({
+        published_at: "2019-01-16",
+      });
+      searchRepo.mockReturnValueOnce({ items: [{ number: 123 }] });
+      getPullRequest.mockReturnValueOnce({
+        data: {
+          number: 123,
+          merge_commit_sha: "1a2b",
+          labels: [{ name: "skip-release" }, { name: "minor" }],
+        },
+      });
+      getGitLog.mockReturnValueOnce(
+        await logParse.normalizeCommits([
+          makeCommitFromMsg("Feature (#124)"),
+          makeCommitFromMsg("I wasn't released previously", {
+            hash: "1a2b",
+          }),
+        ])
+      );
+      exec.mockReturnValueOnce("0");
+      exec.mockImplementationOnce(() => {
+        throw new Error();
+      });
+      exec.mockImplementationOnce(() => {
+        throw new Error();
+      });
+
+      expect(await gh.getCommits("12345", "1234")).toMatchSnapshot();
+    });
+
     test("should include PR opener in authors (in case of external rebase)", async () => {
       const gh = new Release(git);
 

diff --git a/packages/core/src/auto.ts b/packages/core/src/auto.ts
@@ -1192,6 +1192,14 @@ export default class Auto {
 
     await this.setGitUser();
 
+    this.hooks.onCreateLogParse.tap("Omit merges from master", (logParse) => {
+      logParse.hooks.omitCommit.tap("Omit merges from master", (commit) => {
+        if (commit.subject.includes(`Merge origin/${this.baseBranch}`)) {
+          return true;
+        }
+      });
+    });
+
     const currentBranch = getCurrentBranch();
     const initialForkCommit = (
       (

diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts
@@ -4,10 +4,9 @@ import path from "path";
 import { retry } from "@octokit/plugin-retry";
 import { throttling } from "@octokit/plugin-throttling";
 import { Octokit } from "@octokit/rest";
-import gitlogNode from "gitlogplus";
+import { gitlogPromise as gitlog } from "gitlog";
 import { HttpsProxyAgent } from "https-proxy-agent";
 import tinyColor from "tinycolor2";
-import { promisify } from "util";
 import endent from "endent";
 import on from "await-to-js";
 
@@ -21,8 +20,6 @@ import { gt, lt } from "semver";
 import { ICommit } from "./log-parse";
 import { buildSearchQuery, ISearchResult } from "./match-sha-to-pr";
 
-const gitlog = promisify(gitlogNode);
-
 type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>> &
   Partial<Pick<T, K>>;
 
@@ -326,23 +323,40 @@ export default class Git {
         ? await execPromise("git", ["rev-parse", start])
         : "";
 
-      const log = await gitlog<ICommit>({
+      const log = await gitlog({
         repo: process.cwd(),
         number: Number.MAX_SAFE_INTEGER,
         fields: ["hash", "authorName", "authorEmail", "rawBody"],
         // If start === firstCommit then we want to include that commit in the changelog
         // Otherwise it was that last release and should not be included in the release.
         branch: first === startSha ? end : `${start.trim()}..${end.trim()}`,
         execOptions: { maxBuffer: 1000 * 1024 },
+        includeMergeCommitFiles: true,
       });
 
-      return log.map((commit) => ({
-        hash: commit.hash,
-        authorName: commit.authorName,
-        authorEmail: commit.authorEmail,
-        subject: commit.rawBody!,
-        files: (commit.files || []).map((file) => path.resolve(file)),
-      }));
+      return log
+        .map((commit) => ({
+          hash: commit.hash,
+          authorName: commit.authorName,
+          authorEmail: commit.authorEmail,
+          subject: commit.rawBody!,
+          files: (commit.files || []).map((file) => path.resolve(file)),
+        }))
+        .reduce((all, commit) => {
+          // The -m option will list a commit for each merge parent. This
+          // means two items will have the same hash. We are using -m to get all the changed files
+          // in a merge commit. The following code combines these repeated hashes into
+          // one commit
+          const current = all.find((c) => c.hash === commit.hash);
+
+          if (current) {
+            current.files = [...current.files, ...commit.files];
+          } else {
+            all.push(commit);
+          }
+
+          return all;
+        }, [] as ICommit[]);
     } catch (error) {
       console.log(error);
       const tag = error.match(/ambiguous argument '(\S+)\.\.\S+'/);

diff --git a/packages/core/src/release.ts b/packages/core/src/release.ts
@@ -391,9 +391,19 @@ export default class Release {
           });
           released = false;
         } catch (error) {
-          // --is-ancestor returned false so the commit is **before** "from"
-          // so do not release this commit again
-          released = true;
+          try {
+            // --is-ancestor returned false so the commit might be **before** "from"
+            // so test if it is and do not release this commit again
+            // This determines:         Is this commit an ancestor of this commit?
+            //                                       ↓                ↓
+            execSync(`git merge-base --is-ancestor ${commit.hash} ${from}`, {
+              encoding: "utf8",
+            });
+            released = true;
+          } catch (error) {
+            // neither commit is a parent of the other so include it
+            released = false;
+          }
         }
 
         if (released) {

diff --git a/typings/gitlog.d.ts b/typings/gitlog.d.ts